High execution time on number partitioning program in Python 2.7 I am writing a program to count only the partitions of a number with distinct parts.
I am using a bottom-up approach to dynamic programming to generate partition lists from previously obtained partition lists. I think my program runs correctly, as I have tested for some inputs and verified from OEIS. But it's extremely slow for n>15. I think my algorithm has a complexity currently north of O(n^3), but I couldn't think of a better way to do it. Can anyone help with making it faster?

# Logic - The partition of a number 'n', will be 1 + the partition of 'n-1', 2 + the partition of 'n-2', and so on.
# So we start from 1, and build partition lists based on previously gotten partitions
# The loop doesn't have to go all the way till 1.
# For example, for 6, we only have to go till 'n-3' and stop, because after that, we only get duplicate lists. This is not to say that we don't get
# duplicates before, but after 'n-3', we get JUST duplicates
# So we only have to go till n-x >= x

from collections import Counter
compare = lambda x, y: Counter(x) == Counter(y)

def make_partitions(n):
    # Bottom up approach starts at 1 and goes till n, building on partitions already obtained

    # partitions dictionary contains list of lists of partitions
    # Ex - 1: [[1]]
    # Ex - 2: [[2], [1,1]]
    # Ex - 3: [[3], [2,1], [1,1,1]]
    partitions = {}
    start = 1
    while start <= n:
        partitions[start] = []
        # Appending the number itself as a partition of length 1
        partitions[start].append([start])

        # prev stores the number currently being used to build the partition list
        prev = start - 1
        # pp stores all partition lists obtained so far for the current number
        pp = []

        while prev >= start-prev:
            # curr_partitions stores the partition lists that make up the desired number, FROM the current number
            # Ex - for desired number 6, in first loop, it stores those lists which make up 6 from those of 5; in the second loop, from those of 4 and so on
            curr_partitions = []
            prev_partitions = partitions[prev]
            for p in prev_partitions:
                q = list(p)
                q.append(start-prev)

            # self-explanatory. compare function is used to see if the list already exists
            does_exist_already = False
            for ppp in pp:
                if compare(q, ppp):
                    does_exist_already = True
            if not does_exist_already:
                curr_partitions.append(q)

        # We have got the entire list of partitions that make up the desired number FROM the current number, so we add to the dictionary
        partitions[start].extend(curr_partitions)
        prev -= 1
        pp.extend(curr_partitions)
    start += 1
return partitions

def answer(n):
    partitions = make_partitions(n)
    req_partition_list = partitions[n]
    final_partition_list = []
    count = 0

    # This for loop is to weed out lists which contain duplicate values
    for p in req_partition_list:
        c = Counter(p)
        if all(v==1 for v in c.values()):
            final_partition_list.append(p)
            count += 1
    return count

if __name__ == '__main__':
    n = int(raw_input())
    print answer(n)

Indentation

Your code is indented using a mixture of spaces and newlines. In Python, the use of 4 spaces per indent level is preferred. While Python 2 allows for mixed indentation, PEP8 recommends these be converted to spaces exclusively. Your indentation format seems to have affected display of the following lines:

        partitions[start].extend(curr_partitions)
        prev -= 1
        pp.extend(curr_partitions)
    start += 1
return partitions

The resulting code does not run. I have cleaned up indentation for make_partitions:

from collections import Counter
compare = lambda x, y: Counter(x) == Counter(y)

def mp(n):
    partitions = {}
    curr_partitions = []
    start = 1
    while start <= n:
        partitions[start] = []
        partitions[start].append([start])
        prev = start - 1
        pp = []

        while prev >= start-prev:
            curr_partitions = []
            prev_partitions = partitions[prev]
            for p in prev_partitions:
                q = list(p)
                q.append(start-prev)
            does_exist_already = False
            for ppp in pp:
                if compare(q, ppp):
                    does_exist_already = True
            if not does_exist_already:
                curr_partitions.append(q)

            partitions[start].extend(curr_partitions)
            prev -= 1
            pp.extend(curr_partitions)
        start += 1
    return partitions

Partitions

The edited code produces the following output for n = 7:

{1: [[1]], 2: [[2], [1, 1]], 3: [[3], [1, 1, 1]], 4: [[4], [1, 1, 1, 1],[1, 1,
2]], 5: [[5], [1, 1, 2, 1]], 6: [[6], [1, 1, 2, 1, 1], [1, 1, 2, 2], [1, 1, 1, 3
]], 7: [[7], [1, 1, 1, 3, 1], [1, 1, 2, 1, 2], [1, 1, 2, 3]]}

It seems that partitions containing integers in the range n/2 < i < n are not included. Then, it is hard to see how final_partition_list could contain the correct partitions. And, checking small values of n:

n = 3, output = 1

n = 4, output = 1

n = 5, output = 1

n = 6, output = 1

etc.

Your code may not necessarily be broken, but simply a victim of incorrect conversion from mixed indentation to spaces only.


Creating a human-readable list from the values of a map I'm creating a human-readable list from the keys of a map object.  I want it to be comma-separated, but include the word "and" at the end before the last element, supplying a given locale for the word "and".

E.g., my map might have the strings:

"AK", "Alaska"
"CA", "California"
"NY", "New York"

I would want as output:

Alaska, California, and New York

for English and

Alaska, California, y New York

for Spanish.

So I have:

public static String getHumanReadableStates(Map<String, String> stateMap) {

    long count = stateMap.values().stream().map(Object::toString).count();
    return stateMap.values().stream().map(Object::toString)
            .limit(count - 1)
            .collect(Collectors.joining(", "))
            .concat(", ")
            .concat(LocaleUtil.getBundleValue("theWordAnd"))
            .concat(" ")
            .concat(Objects.requireNonNull(stateMap.values().stream().reduce((first, second) -> second)
                    .orElse(null)));
}

where LocaleUtil has a method called getBundleValue() to simply get the word "and" from messages.properties.

Is there a simpler way to achieve this?  Or should streams be avoided altogether?

A stream-based approach is still feasible, by materializing your state names into a List first.

By using a List, you can retrieve all the names except the last using List.subList(int, int) (this may be empty, for a single-entry Map), and then retrieving the last (or the only, for a single-entry Map) directly using a simple List.get(int). Afterwards, you just need to handle for an empty Map or String values.

public static String getHumanReadableStates(Map<String, String> stateMap) {
    List<String> names = new ArrayList<>(stateMap.values());
    if (names.isEmpty()) {
        return "";
    }
    String exceptLastPlusBlank = Stream.concat(
            names.subList(0, names.size() - 1).stream(), Stream.of(""))
            .collect(Collectors.joining(", "));
    return Stream.of(exceptLastPlusBlank, names.get(names.size() - 1))
            .filter(v -> !v.isEmpty())
            .collect(Collectors.joining(LocaleUtil.getBundleValue("theWordAnd") + " "));
}

Stream.of("") is introduced to append the final comma.


Web server to switch GPIO pin I am running a simple Python webserver to toggle a GPIO pin
on a Raspberry Pi. All seems to work well, but I have a few
questions about my code (see below).

I am using the urlparse module to obtain the URL arguments.

The HTML file just sends out Ajax requests using buttons when clicked.

I've commented out the send_response lines, and it seems to work
both ways. Is it mandatory to send a response?
Is the code below easy to exploit? Are there improvements I can make to make it a bit more robust?

Python code:

import time
import SimpleHTTPServer
import SocketServer
from urlparse import *

ledpin=18
myport=8123
# Turn LED on  => http://ip_of_pi:port/control.html?led=on
# Turn LED off => http://ip_of_pi:port/control.html?led=off
class MyRequestHandler(SimpleHTTPServer.SimpleHTTPRequestHandler):
    def do_GET(self):
        urlcomp=urlparse(self.path) # Split URL in components
        query = parse_qs(urlcomp.query) # Get args as dictionary
        if len(query)==0 or query.has_key("led")==False:
            self.path=urlcomp.path
            # self.send_response(200)
        elif query["led"] == ["on"] :
            GPIO.output(ledpin,True)
            # self.send_response(200)
            return
        elif query["led"] == ["off"] :
            GPIO.output(ledpin,False)
            # self.send_response(200)
            return
        SimpleHTTPServer.SimpleHTTPRequestHandler.do_GET(self)

GPIO.setwarnings(False)
GPIO.setmode(GPIO.BOARD)
GPIO.setup(ledpin,GPIO.OUT)

Handler = MyRequestHandler
httpd = SocketServer.TCPServer(('0.0.0.0', myport), Handler)
httpd.serve_forever()

Import mess
You fail to bring GPIO into the namespace by using something along the lines of from RPi import GPIO. You also import time but don't use it. And you import everything from urlparse using from urlparse import *. This form is frowned upon as it polutes your namespace and may override something else. Better use import urlparse or from urlparse import urlparse, parse_qs.
Global layout mess
It is good practice to put top-level code into functions and call them in a if __name__ == '__main__' clause. That way you can start an interactive session and import your script for testing purposes without having code running as a side effect of your import:
def init_GPIO(ledpin=18):
    GPIO.setwarnings(False)
    GPIO.setmode(GPIO.BOARD)
    GPIO.setup(ledpin,GPIO.OUT)

def init_webserver(port=8123):
    httpd = SocketServer.TCPServer(('0.0.0.0', port), MyRequestHandler)
    httpd.serve_forever()

if __name__ == "__main__":
    init_GPIO()
    init_webserver()

Note that I also turned your constants into parameters with default values. In this case it can make testing much more easier.
You also don't need your alias (Handler = MyRequestHandler) and can use the class directly.
Request handler
There is a more idiomatic way of checking if 'led' is in the query string; by using EAFP:
class MyRequestHandler(SimpleHTTPServer.SimpleHTTPRequestHandler):
    def do_GET(self):
        urlcomp = urlparse(self.path) # split url in components
        query = parse_qs(urlcomp.query) # Get args as dictionary
        try:
            led_state = query['led']
        except KeyError:
            pass
        else:
            turn_on = led_state == ["on"]
            GPIO.output(ledpin, turn_on)
        self.send_response(200)

It is recommended that you call the send_response method since, even if the raspberry-pi handled the request and turned the led on or off, your browser is still waiting for a reply of your webserver. Not calling self.send_response(200) should result in a timeout of your browser at some point.
If you want to send back data to the browser on top of the status code, you can write whatever content you want into self.wfile which will stream is to your browser. You can use it to make it simpler to control your raspi. You can also cusomize the message based on how you handled the action.
class MyRequestHandler(SimpleHTTPServer.SimpleHTTPRequestHandler):
    def do_GET(self):
        urlcomp = urlparse(self.path) # split url in components
        query = parse_qs(urlcomp.query) # Get args as dictionary
        try:
            led_state = query['led']
        except KeyError:
            message = "
No commands processed
"
        else:
            if led_state in (["on"], ["off"]):
                GPIO.output(ledpin, led_state == ["on"])
                message = "
Led turned {}
".format(led_state[0])
            else:
                message = "
Unknown action {}
".format(led_state)
        # Build links whatever the action was
        message += """

                      Turn on the led

                      Turn off the led

"""
        self.send_response(200)
        # Custom headers, if need be
        self.send_header('Content-type', 'text/html')
        self.end_headers()
        # Custom body
        self.wfile.write(message)

If you want to read the body from a file, just use regular python file manipulation:
with open('control.html') as html:
    self.wfile.write(html.read())


Queuing e-mail notifications in a background thread I'm new to parallel programming concepts. I'm trying to implement fire-and-forgot kind of method in my project. Somebody please evaluate the below code & let me whether this is thread safe or not.

    //Controller
    public ActionResult AddMail(Guid userId)
    {
        EmailNotification.QueueMailAync(userId);
    }

    //BL
    public static class EmailNotification
    {
    public static void QueueMailAync(Guid userId)
    {
        HostingEnvironment.QueueBackgroundWorkItem
        (
            cancellationToken =>
            {
                try
                {
                    QueueMail(userId);
                }
                catch (Exception e)
                {
                    logger.Error(e);
                }
            }
        );
    }

    static void QueueMail(Guid userId)
    {
        using (var context = new DBEntities())
        {
            var user = context.Users.FirstOrDefault(u => u.Id == userId);

            string body = context.EmailContents.FirstOrDefault().BodyText;
            body = body.Replace("{USER_NAME}",user.UserName);

            var mail = new ArchOver.Model.Email();
            mail.BodyText = body;
            mail.UserId = userId;
            mail.Subject = "Aync";
            mail.EmailTo = user.Person.Email;

            context.Users.Add(mail);
            context.SaveChanges();
        }
    }
}

I would always advocate SingleOrDefault() over FirstOrDefault() if the
  intent of the query is to only expect 0..1 data objects returned.

And to explain further.....

My comments are mainly around the use of FirstOrDefault().  To me your code isn't wrong as such and the use of FirstOrDefault will produce what you are looking for.

However, there are two problems I see with it.

Intent

When I read FirstOrDefault() I read that to mean that there could potentially be multiple items (0..n) returned from a query.  So that means there is a business reason behind this decision and I would expect the rest of the application to be able to adhere and follow this decision without problems.

So in your case I read this line

context.Users.FirstOrDefault(u => u.Id == userId)

to mean something along the lines of

  There might potentially be more than one user in the database with an
  Id that matches the userId supplied.  However I'm only interested in
  the first one so I will ignore all other users.

Masking potential problems

Using FirstOrDefault() from past experience is typically used because developers don't want an exception to be thrown if the criteria constraint is violated i.e. they don't want to see a bug come through.

However this may also mean that an error occurs somewhere further down stream because of this.  You will then find yourself debugging errors that in fact relate to model data violations and might have nothing to do with the code you are debugging.

The problem should ideally be fixed at the point where the model data violations were made, rather than the rest of the application having to worry about it.

Summary

I would always advocate SingleOrDefault() over FirstOrDefault() if the intent of the query is to only expect 0..1 data objects returned.

So use what you expect the data state to be in.  Unless of course you want to handle bad data state yourself and then I would expect error handling to occur if this was violated.


Uploading file for related entity I see that my code smells.  The first file is a controller, and createAction.  But I don't know where to delegate this.

BookController.php - Whole controller class

So this is most 'smelly' code to me:

public function createAction(Request $request) {
    $user = $this->get('security.context')->getToken()->getUser();

    $book = new Book();

    $form = $this->createForm(new BookType($user->getId()), $book);

    $form->handleRequest($request);

    if($form->isValid()) {
        $book->upload();
        $em = $this->getDoctrine()->getManager();
        $em->persist($book);
        $em->flush();

        $covers = $request->files->get('covers');
        foreach ($covers as $cover) {
            $coverObj = new BookCover();
            $coverObj->setFile($cover);
            $coverObj->setBook($book);
            $coverObj->upload();

            $em->persist($coverObj);
        }
        $em->flush();
        return $this->redirect($this->generateUrl('matiit_book_homepage'));
    }

    return $this->render('MatiitBookBundle:Book:new.html.twig', ['form' => $form->createView()]);
}

Rest of files:

new.html.twig
Book.php
BookType.php
BookCover.php

Uploading in entities is the second thing which bothers me.

It would be nice to know what is worth testing here - I have no experience with tests so it would be very useful for me.

BookController.php

I don't see anything to change, the only thing which i could do different is the return line from

 return $this->render('MatiitBookBundle:Book:new.html.twig', ['form' => $form->createView()]);

i would write something like this to make it more clear

return $this->render(
    'MatiitBookBundle:Book:new.html.twig',
    [
        'form' => $form -> createView()
    ]
);

So if one day you need to add another line you will just add a , and the code will still look good.

new.html.twig

Why var $file? Why this $? It remember me something which is related to jQuery... Do you really need this? I would just do var file or choose a more specific name. (no idea right now, file could be ok but what is book_file? a input type="file"?)

Book.php

You use documentation only to specify the type of the fields? You could start with add the documentation of the fields and document all methods (or at least public/protected methods)

I'm wrong or here the indentation is a bit messed up? Could be just a copy/paste bug, anyway from protected $category; it wil look like you start write an inner class. (same for the end of the file)

Fix it if it's in your code.

What is length? I'm not English so maybe i'm wrong but i don't think length is a great name for a Book class (without documentation it's more hard to understand what is it).

It's the book pages? What about totalPages? It's the length (size) of the file in bytes? What about fileSize?

BookType.php

class BookType extends AbstractType

Why not call your class AbstractType as AbstractBookType?

AbstractType could sound as something which is related to variables type, not your class.

I know you use namespaces but i think it's important to make the name of something very clear.

Here you could add documentation too.

BookCover.php

It looks so similar to Book.php

Some methods are similar, maybe Book should have an instance of BookCover inside instead of recreate the methods from zero?

I need more info about it.

protected function getUploadRootDir()
{
   // the absolute directory path where uploaded
   // documents should be saved
   return __DIR__.'/../../../../../web/'.$this->getUploadDir();
}

I'm not a PHP expert, but what if one day you change the location? You should remember to edit this method inside BookCover and inside Book too! Can't you make it in a static field for the two classes?

And you should maintain the /../../../../../ to point in the new location!

Anyway my review is more about style than other.


Python Tic Tac Toe Building Block Function Would appreciate any input on these building blocks functions of tic tac toe.

def display_field(cells):
    print('''\
---------
| {} {} {} |
| {} {} {} |
| {} {} {} |
---------\
'''.format(*cells))

def analyze_field(cells):
    """

    Args:
        cells (str): a string of an XO pattern

    Returns:
        (str): the result of the XO field
    """

    num_x = cells.lower().count('x')
    num_o = cells.lower().count('o')

    rows = [[cells[j] for j in range(i, i + 3)] for i in range(0, 9, 3)]
    columns = [[row[i] for row in rows] for i in range(3)]

    first_diagonal = rows[0][0] + rows[1][1] + rows[2][2]
    second_diagonal = rows[0][2] + rows[1][1] + rows[2][0]

    if abs(num_x - num_o) not in [0, 1]:
        return 'Impossible'

    num_winners = 0
    for row, column in zip(rows, columns):
        if len(set(row)) == 1:
            num_winners += 1
        if len(set(column)) == 1:
            num_winners += 1
    if num_winners > 1:
        return 'Impossible'

    for row in rows:
        if len(set(row)) <= 1:
            return f'{row[0]} wins'

    for column in columns:
        if len(set(column)) <= 1:
            return f'{column[0]} wins'

    if len(set(first_diagonal)) <= 1:
        return f'{first_diagonal[0]} wins'

    elif len(set(second_diagonal)) <= 1:
        return f'{second_diagonal[0]} wins'

    # No winner
    else:
        if '_' in cells or ' ' in cells:
            return 'Game not finished'
        else:
            return 'Draw'

cells = input('Enter cells:')

display_field(cells)

print(analyze_field(cells))

Enter cells:XO_XO_XOX
---------
| X O _ |
| X O _ |
| X O X |
---------
Impossible

Flat array for display

It looks like display_field accepts a one-dimensional array. By the time you're manipulating it with business logic and presenting it back to the user, it should be a two-dimensional array. Rather than one format string, you would then call '\n'.join().

Presentation vs. business logic

In several places your presentation ("Impossible", "X") is all conflated with your business logic. Do not use strings to return status from analyze_field. What to use instead depends on a number of factors; some options are:

Use an Enum
Use a callback object where there is one function per result option

Along similar lines, do not store 'x' and 'o' strings in the cells. You can use an Enum here, perhaps with values PLAYER1/PLAYER2. Strings are unconstrained, and a matter of style/presentation/UI rather than business logic, which should be more verifiable.

Set membership

if abs(num_x - num_o) not in [0, 1]:

can be

if abs(num_x - num_o) not in {0, 1}:

since order does not matter and set lookup has O(1) time complexity. That said, I think this is equivalent to

if not(-1 <= num_x - num_o <= 1):

assuming that num_x and num_o are integral.

Zip

I question this logic:

for row, column in zip(rows, columns):
    if len(set(row)) == 1:
        num_winners += 1
    if len(set(column)) == 1:
        num_winners += 1

I think what you're looking for is a chain, not a zip:

for sequence in chain(rows, columns):
    if len(set(sequence)) == 1:
        num_winners += 1

In other words, there's no point to checking a row and its corresponding column in the same loop iteration.


Should I use unnamed or named functions? I am using jQuery 1.7.2

I have two ways of writing the same code. One way seems more verbose and the other seems more flexible. Is this the only difference? Is there any performance difference? Is there a better way of writing these?

Code sample #1 uses two named functions so they are reusable. It's a bit more code.

Code sample #2 uses two unnamed functions that work only on the hover method. It's less code but not reusable.

Any thoughts or comments?

//  CODE SAMPLE #1

// IPP FUNCTIONS
var openIPP = function openIPP() {
    $IPPNonDefault.stop().animate({"height": OpenHeight}, DownRate);
}
var closeIPP = function closeIPP() {
    $IPPNonDefault.stop().animate({"height": "0px"}, UpRate);
}

// IPP ACTIONS
$IPPWrapper.mouseover(openIPP).mouseout(closeIPP);

//  CODE SAMPLE #2

$IPPWrapper.hover(
    function() {
        $IPPNonDefault.stop().animate({"height": OpenHeight}, DownRate);
    },
    function() {
        $IPPNonDefault.stop().animate({"height": "0px"}, UpRate);
    }
);

So this is redundant.

var openIPP = function openIPP() {
    $IPPNonDefault.stop().animate({"height": OpenHeight}, DownRate);
}

Because both of these functions are the same.

var openIPP = function() {
    $IPPNonDefault.stop().animate({"height": OpenHeight}, DownRate);
}
function openIPP() {
    $IPPNonDefault.stop().animate({"height": OpenHeight}, DownRate);
}

This is the easiest for me to read.

$IPPWrapper.hover(
    function() {
        $IPPNonDefault.stop().animate({"height": OpenHeight}, DownRate);
    },
    function() {
        $IPPNonDefault.stop().animate({"height": "0px"}, UpRate);
    }
);

But if you want more information in case a bug is thrown then use named functions like so.

$IPPWrapper.hover(
    function openIPP() {
        $IPPNonDefault.stop().animate({"height": OpenHeight}, DownRate);
    },
    function closeIPP() {
        $IPPNonDefault.stop().animate({"height": "0px"}, UpRate);
    }
);


Add data of unknown type to a hash I don't see any way to refactor this, but something tells me I'm missing something.

if val.instance_of?(Hash) then
    @fields.has_key?(key) ? @fields[key].merge!(val) : @fields.merge!({key => val})
elsif val.instance_of?(Array) then
    @fields.has_key?(key) ? @fields[key].push(val) : @fields.merge!({key => Array.new.push(val)})
else
    @fields.has_key?(key) ? @fields[key] << val : @fields.merge!({key => val})
end

The point of the method is to take a hash and insert a value of different types if the key doesn't exist:

{:key => "value}, {:key = ["value"]}, or {:key => {:k => "value"}}

or append if it does:

{:key=> "value1value2"}, {:key => [["value1"], ["value2"]], {:key => {:k1 => "value1", :k2 => "value2"}}

it does work as intended, so my issue is thinking that there is likely an easier way to do it.

There are some obvious targets for refactoring, such as Array.new.push(val) instead of [val], but to me this entire code block stinks. The most obvious smell is the type checking necessitated by a lack of duck typing.

Can you explain more about the tensions that lead to this design? I suspect the best solution is further up the context stack than what you have provided.


How do I refactor this responsive background images code? In my latest project I am using fullscreen background images for each page. I decided to write some code to fetch the different thumbnail sizes and then use it as a background for the appropriate viewport size using media queries. I also made sure that in the case of big screens the smallest image loads first so that the visitor is mostly never greeted with a blank or loading background. I set the image as the featured image of the page and then call it using this code-

$args = array(
            'post_type' => 'attachment',
            'numberposts' => 1,
            'post_status' => null,
            'post_parent' => $post->ID,
            'post_mime_type' => 'image'
            );

            $attachments = get_posts( $args );

            if ( $attachments ) :
                foreach ( $attachments as $attachment ) :
                    $img_thelargeArr = wp_get_attachment_image_src( $attachment->ID, "full");
                    $img_medArr = wp_get_attachment_image_src( $attachment->ID, "med");
                    $img_mediumArr = wp_get_attachment_image_src( $attachment->ID, "medium");

                    $img_thelarge = $img_thelargeArr[0];
                    $img_med = $img_medArr[0];
                    $img_medium = $img_mediumArr[0];
                endforeach;
            endif;

And then in the page.php (or whichever template file with full background) itself I added the following style tag to display images providing fallback for bigger screen sizes so that the background loads progressively.

<style media="screen">
 @media (min-width: 1800px) {
body { background: url(<?php echo $img_thelarge; ?>) no-repeat center center fixed;
background-size: cover; }
html.multiplebgs body { background: url(<?php echo $img_thelarge; ?>), url(<?php echo    $img_med; ?>), url(<?php echo $img_medium; ?>);
background-size: cover;
background-repeat: no-repeat; }
}

@media (min-width: 1000px) and (max-width: 1799px) {
body { background: url(<?php echo $img_thelarge; ?>) no-repeat center center fixed;
background-size: cover; }
html.multiplebgs body { background: url(<?php echo $img_thelarge; ?>), url(<?php echo $img_med; ?>), url(<?php echo $img_medium; ?>);
background-size: cover;
background-repeat: no-repeat; }

}

@media (min-width: 400px) and (max-width: 999px) {
body { background: url(<?php echo $img_med; ?>) no-repeat center center fixed;
background-size: cover; }
html.multiplebgs body { background: url(<?php echo $img_med; ?>), url(<?php echo $img_medium; ?>);
background-size: cover;
background-repeat: no-repeat; }
}

@media (max-width: 399px) {
body { background: url(<?php echo $img_medium; ?>) no-repeat center center fixed;
background-size: cover; }
}
</style>

This works as expected but I am using this for a bunch of other page templates as well and was wondering if I can move this code to a single PHP file and reference it from each of these templates. I don't know OOP so need help to figure this out. There is only one variation in one of the templates I am using $post->ID instead of $attachment->ID.

How do I set this up as a function that can be referenced from an external PHP file in the theme folder itself?  Is it possible to use JavaScript to add inline styles instead of a style tag on the pages?  How do I go about this?

If the browser doesn't support the property/value combination in its entirety, it will ignore it.  There's no need to do feature detection for this.

body { background: url(<?php echo $img_med; ?>) no-repeat center center fixed; }
body { background: url(<?php echo $img_med; ?>), url(<?php echo $img_medium; ?>);
background-size: cover;
background-repeat: no-repeat; }

If you care about older Android devices, you'll want to provide prefixed versions of background-size.

Make use of the cascade!  The first media query is no media query at all

body {
    background: url(<?php echo $img_medium; ?>) no-repeat center center fixed;
    -webkit-background-size: cover;
    background-size: cover;
}

@media (min-width: 400px) {
    body {
        background-image: url(<?php echo $img_med; ?>);
        background-image: url(<?php echo $img_med; ?>), url(<?php echo $img_medium; ?>);
    }
}

@media (min-width: 1000px) {
    body {
        background-image: url(<?php echo $img_thelarge; ?>);
        background-image: url(<?php echo $img_thelarge; ?>), url(<?php echo $img_med; ?>), url(<?php echo $img_medium; ?>);
    }
}

/* cut out the 1800px media query since it looked identical to the 1000px one */


Laravel build a route from a model I've created a library, TomHart/laravel-route-from-model, and looking to get a review on it. As well as the usual code review, I'm also looking for feedback from a users point of view, if you were to use the library, is there anything extra you wish it did, anything different etc.

The key class is:

<?php
namespace TomHart\Routing;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Routing\Router;
use Illuminate\Routing\UrlGenerator;
use Symfony\Component\Routing\Exception\RouteNotFoundException;
class RouteBuilder
{
    /**
     * Get the router instance.
     *
     * @return Router
     */
    private function getRouter(): Router
    {
        return app('router');
    }
    /**
     * Get the UrlGenerator.
     *
     * @return UrlGenerator
     */
    private function getUrlGenerator(): UrlGenerator
    {
        return app('url');
    }
    /**
     * This allows a route to be dynamically built just from a Model instance.
     * Imagine a route called "test":
     *      '/test/{name}/{id}'
     * Calling:
     *      routeFromModel('test', Site::find(8));
     * will successfully build the route, as "name" and "id" are both attributes on the Site model.
     *
     * Further more, once using routeFromModel, the route can be changed. Without changing the call:
     *      routeFromModel('test', Site::find(8));
     * You can change the route to be:
     *      '/test/{name}/{id}/{parent->relationship->value}/{slug}/{otherParent->value}'
     * And the route will successfully change, as all the extra parts can be extracted from the Model.
     * Relationships can be called and/or chained with "->" (Imagine Model is a Order):
     *      {customer->address->postcode}
     * Would get the postcode of the customer who owns the order.
     *
     * @param string  $routeName The route you want to build
     * @param Model   $model     The model to pull the data from
     * @param mixed[] $data      Data to build into the route when it doesn't exist on the model
     *
     * @return string The built URL.
     */
    public function routeFromModel(string $routeName, Model $model, array $data = [])
    {
        $router = $this->getRouter();
        $urlGen = $this->getUrlGenerator();
        $route = $router->getRoutes()->getByName($routeName);
        if (!$route) {
            throw new RouteNotFoundException("Route $routeName not found");
        }
        $params = $route->parameterNames();
        foreach ($params as $name) {
            if (isset($data[$name])) {
                continue;
            }
            $root = $model;
            // Split the name on -> so we can set URL parts from relationships.
            $exploded = collect(explode('->', $name));
            // Remove the last one, this is the attribute we actually want to get.
            $last = $exploded->pop();
            // Change the $root to be whatever relationship in necessary.
            foreach ($exploded as $part) {
                $root = $root->$part;
            }
            // Get the value.
            $data[$name] = $root->$last;
        }
        return rtrim($urlGen->route($routeName, $data), '?');
    }
}

Laravel coupling

What a shame that you decided to tie this to laravel. You can decouple the entire library from laravel framework and only provide a bundle for laravel.

routeFromModel accepts Model, but it can actually work for any object.

$route = $router->getRoutes()->getByName($routeName);
if (!$route) {
  throw new RouteNotFoundException("Route $routeName not found");
}
$params = $route->parameterNames();

This means you really don't need the router, you just need something that gives you an "array of prameter names" based on a "name".

return rtrim($urlGen->route($routeName, $data), '?');

Returning just the data here would make it more flexible.

IoC

You are pulling the RouteBuilder from DI container, why not have the container inject those deps. The way it is now, it could just be a static class with only static methods...


Loading objects for users, roles, and groups from a query with LEFT OUTER JOINs I have this function that returns me a list of users with their roles and groups. As you can see, this is how I fetch and create list of objects. I'm wondering whether this is a good approach and what parts should be improved. I'm not that experienced with PHP so I would appreciate code samples.

I'm also wondering how good an approach it is to first get all users and then make another prepared statement to get user roles and groups. That would mean I will have big number of database calls, so I think it's a bad idea.

    $stmt = $mysqli->prepare("SELECT u.id, u.firstName, u.lastName, u.email,
        u.phoneNumber, u.address, u.birthDate, ur.roleName, cg.id, cg.name FROM users as u
        LEFT OUTER JOIN user_role as ur ON u.id = ur.userId
        LEFT OUTER JOIN user_group as ug on ug.userId = u.id
        LEFT OUTER JOIN control_group as cg on cg.id = ug.groupId WHERE u.id != ?");
    $stmt->bind_param("i", $_SESSION["id"]);
    $stmt->execute();
    $stmt->bind_result($id, $firstName, $lastName, $email, $phoneNumber,
        $address, $birthDate, $roleName, $groupId, $groupName);
    $users = array();

    while ($stmt->fetch()) {
        if (empty($users[$id])) {
            $users[$id] = array(
                'id' => $id,
                'firstName' => $firstName,
                'lastName' => $lastName,
                'email' => $email,
                'phoneNumber' => $phoneNumber,
                'address' => $address,
                'birthDate' => $birthDate,
                'roles' => array(),
                'groups' => array()
            );
        }
        if ($roleName) {
            $found = false;
            foreach ($users[$id]['roles'] as $role) {
                if($role['roleName'] == $roleName){
                    $found = true;
                    break;
                }
            }
            if($found == false)
                $users[$id]['roles'][] = array(
                    'roleName' => $roleName
                );
         }

        if ($groupId) {
            $found = false;
            foreach ($users[$id]['groups'] as $group) {
                if($group['groupName'] == $groupName){
                    $found = true;
                    break;
                }
            }
            if($found == false)
                $users[$id]['groups'][] = array(
                    'groupName' => $groupName
                );
         }
    }

    $stmt->close();
    $mysqli->close();
    echo json_encode($users);

This is the response I get, only thing that I want improve is item index, as you can see in my example as index I have item id, I would like to get correct index starting from 0.

For the exact case you posted here, I would suggest to use group_concat() in the query. It will dramatically reduce the amount of code required, as well as the amount of data delivered from database.

Besides, I see a very little use for the prepared statement here, so the amount of code could be dramatically reduced on this account as well.

$sql = "SELECT u.id, u.firstName, u.lastName, u.email, u.phoneNumber,
    u.address, u.birthDate,group_concat(ur.roleName), roles group_concat(cg.name) groups
    FROM users as u
    LEFT OUTER JOIN user_role as ur ON u.id = ur.userId
    LEFT OUTER JOIN user_group as ug on ug.userId = u.id
    LEFT OUTER JOIN control_group as cg on cg.id = ug.groupId
    GROUP BY u.id";
$res = $mysqli->query($sql);
$users = array();
while ($row = $res->fetch_assoc()) {
    if ($row['id'] == $_SESSION["id"]) {
        continue;
    }
    $row['groups'] = explode(",", $row['groups']);
    $row['roles'] = explode(",", $row['roles']);
    $users[] = $row;
}

This is the response example which I consider most usable and clear.
But note it would take no trouble to make it exactly as in your example above, if you wish.

On a second thought, thinking one move ahead, I suppose you may wish to make groups and roles some sort of interactive - allowing clicks or other interactions with them. And for this purpose you will need group and roled IDs inevitably.

Therefore, to make this code robust and usable in the real life app, you have to supply group and role id as well. For this purpose I can't help it but use PDO, as its helper methods do A LOT of your job. For example, PDO::FETCH_KEY_PAIR gives you nice key-value pairs right out of SQL query:

$groups = $pdo->query("SELECT id, name FROM control_group")->fetchAll(PDO::FETCH_KEY_PAIR);
$roles  = $pdo->query("SELECT id, roleName FROM user_role")->fetchAll(PDO::FETCH_KEY_PAIR);

$sql = "SELECT u.id, u.firstName, u.lastName, u.email, u.phoneNumber,
    u.address, u.birthDate,group_concat(ur.roleName), roles group_concat(cg.name) groups
    FROM users as u
    LEFT OUTER JOIN user_role as ur ON u.id = ur.userId
    LEFT OUTER JOIN user_group as ug on ug.userId = u.id
    LEFT OUTER JOIN control_group as cg on cg.id = ug.groupId
    GROUP BY u.id";
$res = $pdo->query($sql);
$users = array();
while ($row = $res->fetch(PDO::FETCH_ASSOC)) {
    if ($row['id'] == $_SESSION["id"]) {
        continue;
    }

    $user_groups = array();
    foreach(explode(",", $row['groups'] as $id) {
        $user_groups[$id] = $groups[$id]
    }
    $row['groups'] = $user_groups;

    $user_roles = array();
    foreach(explode(",", $row['roles']) as $id) {
        $user_roles[$id] = $roles[$id]
    }

    $row['roles'] = $user_roles;
    $users[] = $row;
}

This is a response example.

As you can see, here we're first selecting all the group and role names, while in the main query selecting only ids. Then in the short loop finally creating groups and roles  arrays.


Next and Previous navigation I'm sorry, maybe this question is really stupid but I have really been thinking a lot about it recently, thinking of which type of <prev | next> web navigation approach is better. FYI the stack is React + Redux.

With using name properties (or also we can use something like custom boolean field forward="ture" and check it)

HTML:

      <button name="prev" onClick={this.onYearChange.bind(this)}>Prev</button>

{this.state.year}

      <button name="next" onClick={this.onYearChange.bind(this)}>Next</button>

JS handler:

 onYearChange(e){
        let dir = (e.target.name === 'prev') ? -1 : 1,
            year = this.props.selectedYear + dir;

        this.props.onYearChangeHandler(year);
  }

With passing custom property dir with inc/dec value

HTML:

   <button dir={-1} onClick={this.onYearChange.bind(this)}>Prev</button>

{this.state.year}

   <button  dir={1} onClick={this.onYearChange.bind(this)}>Next</button>

JS:

onYearChange(e){
        let dir = e.target.dir,
            year = this.props.selectedYear + dir;

        this.props.onYearChangeHandler(year);
}

Or with two separate binded functions

HTML:

     <button onClick={this.toPrevYear.bind(this)}>Prev</button>

{this.state.year}

     <button onClick={this.toNextYear.bind(this)}>Next</button>

JS:

    toPrevYear(e) {
        this.onYearChange(-1);
    };

    toNextYear(e) {
        this.onYearChange(1);
    };

    onYearChange(val) {
        let year = this.props.selectedYear + val;

        this.props.onYearChangeHandler(year);
    }

Or is there some better solution for this scenario?

Think your worrying yourself about small details here, any advantages of one over the other would be minor at best.

I think out of all 3, the last solution would be my preferred one. It's clean from a code separation POV and does exactly what it says on the tin, also easier to change behaviour without affecting the other should that ever need to happen.

FWIW for performance reasons it's recommended you bind UI event handlers in the constructor e.g.

constructor() {
    this.toPrevYear = this.toPrevYear.bind(this);
}
...
render() {
    return <button onClick={this.toPrevYear}>Prev</button>;
}


Java API token expiration handler I want to implement a utility class which manages tokens generated by any API. The idea is simple:

  It should store the current token value. The token should be initialized just before first use and then re-initialized after some time has elapsed. If the API returns an error related to the token, it should be regenerated and job should be repeated.

Here we have the utility class:

import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;

public abstract class TokenManager {

    private volatile String token;
    private ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor();
    private ScheduledFuture<?> schedule;

    public void useToken(TokenConsumer tokenConsumer) {
        useToken(tokenConsumer, true);
    }

    private void useToken(TokenConsumer tokenConsumer, boolean retryWhenTokenExpired) {
        try {
            tokenConsumer.useToken(getToken());
        } catch (Exception e) {
            if (retryWhenTokenExpired && tokenExpired(e.getMessage())) {
                System.out.println("token expired, retry");
                resetToken();
                useToken(tokenConsumer, false);
            }
        }
    }

    synchronized private String getToken() {
        if (token == null) {
            resetToken();
        }
        return token;
    }

    synchronized private void resetToken() {
        if (schedule != null) {
            schedule.cancel(false);
        }
        System.out.println("reset token");
        token = generateToken();
        int oneMinute = 60 * 1000;
        long delay = getTokenValidityTimeMillis() - oneMinute;
        schedule = scheduler.schedule(this::resetToken, delay > 0 ? delay : getTokenValidityTimeMillis(), TimeUnit.MILLISECONDS);

        System.out.println("reset token finished");
    }

    protected abstract String generateToken();

    protected abstract boolean tokenExpired(String message);

    protected abstract long getTokenValidityTimeMillis();

    @FunctionalInterface
    public interface TokenConsumer {
        void useToken(String token) throws Exception;
    }
}

And here we have its simple implementation with use example:

import java.util.Random;

public class TokenManagerImpl extends TokenManager {
    @Override
    protected String generateToken() {
        //here would go some api call which retrieves token
        try {
            Thread.sleep(2000);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        return "token " + new Random().nextInt();
    }

    @Override
    protected boolean tokenExpired(String message) {
        return "Token expired".equals(message);
    }

    @Override
    protected long getTokenValidityTimeMillis() {
        return 1000;
    }

    public static void main(String[] args) {
        TokenManager tokenManager = new TokenManagerImpl();
        TokenConsumer tokenConsumer = token -> {
            //here would go some api call which uses token
            System.out.println("using token : " + token);
        };
        while (true) {
            tokenManager.useToken(tokenConsumer);
            try {
                Thread.sleep(50L);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }
}

useToken

I don't really get your expected behavior from this method. You only retry when expired, what if an exception occured while not yet expired? Nothing happens, not even a log. The same with exceptions that occur when retryWhenTokenExpired is not set, the exception gets swallowed without any trace.

  catch (Exception e) {
    if (retryWhenTokenExpired && tokenExpired(e.getMessage())) {
        System.out.println("token expired, retry");
        resetToken();
        useToken(tokenConsumer, false);
    }
  }

I would make sure the following constraint is true: either a retry is performed or an exception is propagated up the stack. In addition, on expiration, reset the token when requested so. log is an abstraction of your System.out.println.

  catch (Exception e) {
    log("error using token: " + e.getMessage());
    if (!retryWhenTokenExpired) {
        throw e;
    }
    final boolean expired = tokenExpired(e.getMessage());
    if (expired) {
        log("token expired, retry");
        resetToken();
    }
    useToken(tokenConsumer, !expired);
  }


Handling errors of nested promises This is in one of my AngularJS controllers:

websiteService.get('websites').then(function(data) {
    $scope.websites = data;
    websiteService.get('groups').then(function(data) {
        $scope.groups = data;
        websiteService.get('websites_groups').then(function(data) {
            $scope.websites_groups = data;
            // If everything goes well, my code will continue here - not really neat
        }, function(error) {
            $scope.errors.push(error);
        });
    }, function(error) {
        $scope.errors.push(error);
    });
}, function(error) {
    $scope.errors.push(error);
});

What would be a better, more elegant way to do this? Is there a best practice?

In my opinion, success functions are nested really nicely, in obvious and chronological and logical order, while error functions make everything messy.

If I understand correctly, you can unnest your code, and catch any possible error at the end. That's the idea of using promises:

websiteService.get('websites').then(function(data) {
  $scope.websites = data
  return websiteService.get('groups')
}).then(function(data) {
  $scope.groups = data
  return websiteService.get('websites_groups')
}).then(function(data) {
  $scope.websites_groups = data
}).catch(function(error) {
  $scope.errors.push(error)
})


nodejs grunt plugin using async module I'm looking for some code review of my usage of async. Mainly why this does not work without the done = this.async() method and why I don't have to invoke 'done()' on every request to download a file. This is working now but it is a bit of magic to me. Hoping to understand why it's working and if I'm abusing something.

   /*
   * grunt-esri-slurp
   * https://github.com/steveoh/grunt-esri-slurp
   *
   * Copyright (c) 2014 steveoh
   * Licensed under the MIT license.
   */

    'use strict';
    var fs = require('fs'),
    path = require('path'),

    async = require('async'),
    mkdirp = require('mkdirp'),
    request = require("request"),
    S = require('string'),

    unwind = require('./unwinder'),
    esriModules = require('./esriModules');

  module.exports = function(grunt) {
    grunt.registerTask('esri_slurp', 'download esri js api amd modules and create a package', function() {
      var options = this.options({
          packageLocation: 'src/esri/',
          version: '3.9'
        }),
        done = this.async();

      options.packageLocation = S(options.packageLocation).ensureRight('/').s;

      grunt.log.subhead('downloading esri version ' + options.version + ' modules');

      mkdirp.sync(options.packageLocation);

      var esriVersionBaseUrl = 'http://js.arcgis.com/' + options.version + 'amd/js/esri/';
      grunt.verbose.writeln('esri base url: ' + esriVersionBaseUrl);

      async.eachLimit(esriModules, 10, function(file, callback) {
        var subPath = file.substr(0, file.lastIndexOf('/') + 1),
          fileFolder = options.packageLocation + subPath,
          fileName = path.basename(file),
          httpUrl = esriVersionBaseUrl + subPath + fileName;

        if (!fs.existsSync(fileFolder)) {
          grunt.verbose.writeln(['creating folder ' + fileFolder]);

          mkdirp.sync(fileFolder);
        }

        grunt.verbose.writeln(['requesting ' + httpUrl]);

        request({
            uri: httpUrl,
            encoding: 'binary'
          },
          function(error, response, body) {
            if (body && body.length > 0) {
              grunt.verbose.or.write('.');
              grunt.verbose.writeln(['writing: ' + options.packageLocation + file]);

              var f = S(file);
              if (f.endsWith('.js') || f.endsWith('.css')) {
                body = unwind(body);
              }

              fs.writeFile(options.packageLocation + file, body, 'binary');
            }

            callback(error, body);
          });
      });
    });
  };

this.async() tells grunt that you're doing something async and that it should wait until you call the done method. Usually it just goes to the next grunt task when the execution is done. Since you're using the async module you can call the done method in the third argument to async.eachLimit which is supposed to be a function that is called when all the work is done.

// tell grunt we're doing something async
var done = this.async();

async.eachLimit(['unicorn', 'rainbows'], 10, function(file, cb) {
    // do something async
    cb(); // tell `async` we're done doing work for this item
}, function (err) { // called when all the items have called their `cb()`
    if (err) {
        grunt.warn(err);
    }

    // all the items are done, let's tell grunt we are done
    done();
});


Python update SQLite DB Values At the moment I try to get used to SQLite and Python. I started with insert, select and so on. But now i want to get a bit further.
Now I have a DB with 200 rows for testing and in want to update two values of each row in this case called "Test2" and "Test3". My problem is that those two values should be the value that they have +200 and +400 so i have to make an select before updating them (at least with my knowledge).

I added a simple time measurement to my code to see if i will be able to make my code more efficient and faster!

At the moment it takes 0.7344 seconds for me to edit 200 rows of data. Which is pretty slow! Especially if i think to do it for bigger DB's.

Things I want to achieve:

Is it really necessary to make the select or could I use the present Data in any other way?
Make things faster! Is it possible to edit multiple rows at the same time? Like parallelized tasks?

Any other tips are welcome, as i try to improve myself!

Here is my code so far:

import sqlite3
import time

def database_test():
    # Open Database
    conn = sqlite3.connect('SQLite_Test.db')
    c = conn.cursor()

    i = 0

    for i in range(200):
        c.execute('SELECT Test2, Test3 FROM Test WHERE Test1 = ?',  (i,))

        DB_Values = []
        DB_Values = c.fetchone()

        Value1 = DB_Values[0]+200
        Value2 = DB_Values[1]+400

        c.execute('''UPDATE Test SET Test2 = ?, Test3 = ? WHERE Test1= ?''', (Value1, Value2, i))
        i += 1

    # Save (commit) the changes
    conn.commit()

start_time = time.time()
database_test()
print("--- %s seconds ---" % round((time.time() - start_time),4))

SQL (and thus SQLite) support a variety of operators suitable (amongst others) in UPDATE statements.

This means that you can simplify your whole loop into a single statement:

query = '''
    UPDATE Test SET Test2 = (Test2 + ?), Test3 = (Test3 + ?)
    WHERE Test1 >= 0 AND Test1 < 200
'''
c.execute(query, (200, 400))

Also you should take the habit to use perf_counter instead of time to measure elapsed time. Or use the dedicated timeit module.


Work command handler for an economy system I have been working on this code, which is the part of an economy system that handles the work command. This lets the user pick a role and if they have enough level then they will get it by updating a db.

I know it's inefficient and I am unsure how to reduce the if count and keeping a cyclomatic complexity below 15. I need to add 8 more jobs but I can't add anymore.

How can I reduce the if count?
What else have I done wrong?

The code I have been working on

  @job.command()
  async def apply(self,ctx,*,selection:str):
    a = banned(ctx.author.id,self.bot.blacklisted)
    if a is True:
      return await ctx.send("You are blacklisted from using this command.")
    db = client.bot
    posts = db.user
    for x in posts.find({"guildid":f"{ctx.guild.id}","id":f"{ctx.author.id}"},{ "_id": 0,"job":1,"level":1}):
      level = int(x["level"])
    if selection.lower() == "garbage collector" and level >= 0:
      posts.update_one({"id":f"{ctx.author.id}","guildid":f"{ctx.guild.id}"}, {"$set": { "job": "Garbage Collector","income":50}})#save
    elif selection.lower() == "cleaner" and level >= 7:
      posts.update_one({"id":f"{ctx.author.id}","guildid":f"{ctx.guild.id}"}, {"$set": { "job": "Cleaner","income":70}})#save
    elif selection.lower() == "car washer" and level >= 16:
      posts.update_one({"id":f"{ctx.author.id}","guildid":f"{ctx.guild.id}"}, {"$set": { "job": "Car Washer","income":105}})#save
    elif selection.lower() == "maid" and level >= 27:
      posts.update_one({"id":f"{ctx.author.id}","guildid":f"{ctx.guild.id}"}, {"$set": { "job": "Maid","income":145}})#save
    elif selection.lower() == "receptionist" and level >= 36:
      posts.update_one({"id":f"{ctx.author.id}","guildid":f"{ctx.guild.id}"}, {"$set": { "job": "Receptionist","income":185}})#save
    else:
      return await ctx.send(f"You are currently {level}, please go to school using ?learn")
    await ctx.send(f"You are now working as a {selection.lower()}")

The pymongo document format

"id":f"{message.author.id}"
"name":f"{message.author.name}"
"guildid":f"{message.guild.id}"
"cash":0
"job":"Homeless"
"balance":0
"shares":0
"level":0
"income":0

Why loop?

for x in posts.find({"guildid":f"{ctx.guild.id}","id":f"{ctx.author.id}"},{ "_id": 0,"job":1,"level":1}):
  level = int(x["level"])

You're looping over the result of posts.find to... convert an element to an integer and then throw it away? Are you sure that this is what you want to be doing?

If you care about level, your current code is overwriting it on every iteration, so you'll only get the last one.

Don't repeat yourself

for job_name, level_min, income in (
   ('Garbage Collector',  0,  50),
   (          'Cleaner',  7,  70),
   (       'Car Washer', 16, 105),
   (             'Maid', 27, 145),
   (     'Receptionist', 36, 185)
):
    if selection.lower() == job_name.lower() and level >= level_min:
        data = {
           "id": f"{ctx.author.id}",
           "guildid": f"{ctx.guild.id}"
        }
        set_stuff = {
          "$set": {
              "job": job_name,
              "income": income
          }
        }
        posts.update_one(data, set_stuff)
        break
else:
    return await ctx.send(f"You are currently {level}, please go to school using ?learn")

If you want to get fancier, you can use a case-insensitive dictionary, but for this, that isn't strictly needed.


Setting responses on survey results The below code feels ugly, how can I make it better? (The part i don't like is the ||=. I also don't know if I like the resulting data structure. (An array of hashes)

def survey_results
    @survey_results = []
    unless params[:step_survey_id].blank?
      SurveyResult.find_by_step_survey_id(params[:step_survey_id]).step_survey.step_survey_questions.each do |survey_question|
          survey_result = {:question => survey_question.value}
          answers = {}
          survey_question.survey_result_answers.each do |survey_result_answer|
            #Setup initial answer hash with key as the response, only used when needed
            answers[survey_result_answer.response] ||= 0
            answers[survey_result_answer.response] += 1
        end
        survey_result[:answers] = answers
        @survey_results.push(survey_result)
      end
    end
  end

Regarding the data structure: In cases like this where you always use the same keys for the hashes (i.e. :answers and :question in this case), it is often a good idea to use a simple class instead, which has accessors for the given properties. You can use Struct to easily create such classes. In this case this would look like this:

SurveyResultInformation = Struct.new(:question, :answers)

Using a class instead of a hash is more robust as mistyped attribute names will cause an exception. It is also more extensible as it allows you to add methods to the class - like for example most_common_answer.

However in this particular case where the method is called survey_results and there exists a class called SurveyResult one has to wonder whether survey_results couldn't/shouldn't return instances of the existing SurveyResult class instead.

My guess is you should be returning an array of SurveyResult objects and extend that class to offer the functionality you need. Though without knowing more of your code it's hard to say for sure.

At the very least you should rename the method or the class to avoid the impression that the method returns instances of that class.

Now to your code:

First of all your method names sounds like it would return the collected survey results, but in fact it just stores them in an instance variable and apparently returns nothing of consequence. You should either make it return the results (possible caching them in the instance variable) or change its name to something imperative-sounding like initialize_survey_results instead.

You're creating an empty array and then appending to it once per iteration in an each-loop. If you do this you can (and should) almost always use map instead.

answers[survey_result_answer.response] ||= 0
answers[survey_result_answer.response] += 1

As you already pointed out this is a bit ugly. Luckily ruby's Hash class has a feature that gets rid of the need for ||= here: It allows you to set default values which will be returned instead of nil if a key was not found. If we replaces answers = {} with answers = Hash.new(0) to set the default value to 0, you can just get rid of the ||= line.

SurveyResult.find_by_step_survey_id(params[:step_survey_id]).step_survey

This line stumped be a bit when I first read it. It is not apparent why you're not just calling StepSurvey.find(step_survey_id). First I thought maybe you don't want to get the step survey when there is no survey result pointing to it, but in that case the above code would cause an exception, so that can't be it.

If I were to write this, I would do something like this:

def survey_results
  if params[:step_survey_id].blank?
    []
  else
    StepSurvey.find(params[:step_survey_id]).step_survey_questions.map do |survey_question|
      survey_result = SurveyResultInformation.new(survey_question.value)
      answers = Hash.new(0)
      survey_question.survey_result_answers.each do |survey_result_answer|
        answers[survey_result_answer.response] += 1
      end
      survey_result.answer = answers
      survey_result
    end
  end
end


Group similar objects into array of arrays I have the following code, that groups similar todos into an array of arrays, using lodash (https://lodash.com). Here is a plnkr http://plnkr.co/edit/t4J5bKGjMlQxW2z9Zh7x?p=preview (open your console and look into script.js)

function Todo(value) {
  this.value = value;
  this.isSimilar = function(todo) {
    return this.value == todo.value;
  };
}

var _data = [
  new Todo("hello"),
  new Todo("world"),
  new Todo("foo"),
  new Todo("bar"),
  new Todo("hello")
];

var processedTodos = [];
var groups = _.compact(_.map(_data, function(todo) {
  if(processedTodos.indexOf(todo)>-1) {
    return;
  }
  var similarTodos = _.filter(_data, function(todo2) {
    return todo!==todo2 && todo.isSimilar(todo2);
  });
  similarTodos.unshift(todo); // prepend todo to front of array
  [].push.apply(processedTodos, similarTodos);
  return similarTodos;
}));

console.log(groups);

groups contains an array of arrays, where similar todos are in the same array.

For example, when I input this (these are JS objects, but for the sake of readability in JSON):

[
  {"value":"hello"},
  {"value":"world"},
  {"value":"foo"},
  {"value":"bar"},
  {"value":"hello"}
]

It returns this:

[
  [{"value":"hello"},{"value":"hello"}],
  [{"value":"world"}],
  [{"value":"foo"}],
  [{"value":"bar"}]
]

It groups together similar objects.

But the whole code doesn't feel right, there must be a better way to do this!

Another bad thing is that I need an additional array to store the already checked todos (which avoid me duplicate arrays like [todo1,todo2] and [todo2,todo1].

Last but not least, I have to remove undefined values from the array with _.compact, because this line returns undefined when the todo was already processed:

  if(processedTodos.indexOf(todo)>-1) {
    return;
  }

Any idea how to improve this code?

First, using a constructor is overkill. Your Todos don't use inheritance nor share methods (the method is created per instance, thus not shared). You can simply have a factory (a function that returns an object). isSimilar could be just a helper function that has nothing to do with Todo.

function createTodo(value){
  return {
    value: value,
  };
}

var _data = [
  createTodo("hello"),
  createTodo("world"),
  createTodo("foo"),
  createTodo("bar"),
  createTodo("hello")
];

So now, you have a list of objects. You can then use reduce and build up a hash of objects with the value as their key, and an array as the value.

var todosByValue = _data.reduce(function(hash, todo){
  if(!hash.hasOwnProperty(todo.value)) hash[todo.value] = [];
  hash[todo.value].push(todo);
  return hash;
}, {});

// now we have something like:
{
  hello: [{ value: 'hello' }, { value: 'hello' }],
  world: [{ value: 'world' }],
  ...
}

All you need to do is just pull the values out to an array using Object.keys and array.map.

var groupedByTodo = Object.keys(todosByValue).map(function(key){
  return todosByValue[key];
});

In all, it should simply look like this:

function createTodo(value){
  return {
    value: value,
  };
}

var _data = [
  createTodo("hello"),
  createTodo("world"),
  createTodo("foo"),
  createTodo("bar"),
  createTodo("hello")
];

var todosByValue = _data.reduce(function(hash, todo){
  if(!hash.hasOwnProperty(todo.value)) hash[todo.value] = [];
  hash[todo.value].push(todo);
  return hash;
}, {});

var groupedByTodo = Object.keys(todosByValue).map(function(key){
  return todosByValue[key];
});


Hash table solution to twoSum I try the most to solve a twoSum problem in leetcode

  Given an array of integers, return indices of the two numbers such that they add up to a specific target.

  You may assume that each input would have exactly one solution, and you may not use the same element twice.

  Example:

  Given nums = [2, 7, 11, 15], target = 9,

  Because nums[0] + nums[1] = 2 + 7 = 9,
  return [0, 1].

The plan:

brute force to iterate len(nums)   O(n)
search for target - num[i] with a hash table   O(1)

Implement

class Solution:
    def twoSum(self, nums: List[int], target: int) -> List[int]:
        nums_d = {}
        for i in range(len(nums)):
            nums_d.setdefault(nums[i], []).append(i)

        for i in range(len(nums)):
            sub_target = target - nums[i]
            nums_d[nums[i]].pop(0) #remove the fixer
            result = nums_d.get(sub_target)#hash table to search

            if result:
                return [i, result[0]]
        return []

I strives  hours for this solution but found that answer accepted but not passed Score 60.

  Runtime: 60 ms, faster than 46.66% of Python3 online submissions for Two Sum.
  Memory Usage: 16.1 MB, less than 5.08% of Python3 online submissions for Two Sum.

I want to refactor the codes  so that to achieve at least faster than 60%.

Could you please provide hints?

First some stylistic points

nums_d.setdefault(nums[i], []).append(i)

The setdefault is unnecessary here, you can assign a list normally

nums_d[nums[i]] = [i]

When you need both the index and the element use enumerate see PEP279

nums_d = {}
for i in range(len(nums)):
    nums_d.setdefault(nums[i], []).append(i)

nums_d = {}
for i, e  in enumerate(nums):
    nums_d[e] = [i]

Use comprehension when possible (They use the C style looping and is considered to be faster)

nums_d = { e: [i] for i, e  in enumerate(nums) }

Hint

You loop over nums twice, but this can be done in one loop! To make it O(n)

Whenever you visit a new element in nums ->

Check if it's sum complement is in nums_d, else add the target - element to the dictionary with the index as value t - e : i

   nums_d = {}
for i, e in enumerate(nums):
    if e in nums_d:
        return [nums_d[e], i]
    nums_d[target - e] = i


Using ZF2 Event manager to save a model I have been reading about ZF2 EventManager for some time now and I wonder what you'd think about this approach.

class Model_Member extends App_Model_Model
{
    protected $_data = array(
        'ID' => null,
        'UserName' => null,
        'Password' => null,
        'EmailAddress' => null,
        'CreatedOn' => null,
        'Status' => 0,
        'PasswordResetHash' => null,
        'Role' => 'member'
    );
    const EVENT_SAVE = 'save';
    const EVENT_POSTSAVE = 'post.save';
    public function init()
    {
        $this->events()->attach(self::EVENT_SAVE, array('Service_Member', 'save'));
    }
    public function save()
    {
        $id = $this->events()->trigger(self::EVENT_SAVE, $this)->last();
        $this->ID = $id;
        $this->events()->trigger(self::EVENT_POSTSAVE, $this);
    }
}

And of course there is Service_Member

public function save($event)
{
    // Get the member from the event
    $member = $event->getTarget();
    // If there is no id, save, otherwise update
    if (!$member->ID) {
        $id = $this->doSave($member);
        $event->setParam('id', $id);
        return $id;
    } else {
        return $this->doUpdate($member);
    }
}
public function doSave(Model_Member $member)
{
    // Check if username is taken
    if ($this->checkIfExists('UserName', $member->UserName)) {
        throw new App_Exception_UserNameExists('This username is already taken.');
    }

    // Check if email address already registered
    if ($this->checkIfExists('EmailAddress', $member->EmailAddress)) {
        throw new App_Exception_EmailAddressExists('This email is already registered.');
    }

    // Set default values
    $member->CreatedOn = date('Y-m-d H:i:s');

    // Encrypt password
    $member->Password = sha1($member->Password);

    // Get the data and save. Return the member with new ID
    $data = $member->_toArray();
    unset($data['ID']);
    $this->getWriteDb()->insert('members', $data);
    $id = $this->getWriteDb()->lastInsertId();
    return $id;
}

Do you think this is an acceptable approach?

I haven't used PHP recently, just a few, mostly minor notes:

// Encrypt password
$member->Password = sha1($member->Password);

You might want to use a salt here to make your hashing stronger.

$this->getWriteDb()->insert('members', $data);
$id = $this->getWriteDb()->lastInsertId();

If that's possible, insert could return the ID. It would eliminate temporal coupling (users can't call the methods in the wrong order).

(Clean Code by Robert C. Martin, G31: Hidden Temporal Couplings, p302)

'Status' => 0,

0 is a magic number here. What could it mean? A named constants would express the intent.
These comments are rather redundant, the code is clear and says the same:

// Check if username is taken
if ($this->checkIfExists('UserName', $member->UserName)) {
    throw new App_Exception_UserNameExists('This username is already taken.');
}

// Check if email address already registered
if ($this->checkIfExists('EmailAddress', $member->EmailAddress)) {
    throw new App_Exception_EmailAddressExists('This email is already registered.');
}

...

// Get the member from the event
$member = $event->getTarget();

(Clean Code by Robert C. Martin, Redundant Comments, p60)
Btw, are you sure that you need two different type of exceptions here? Wouldn't be the same exception type with different messages enough?
Use private instead of protected if possible:

protected $_data = array(

Should I always use the private access modifier for class fields?; Item 13 of Effective Java 2nd Edition: Minimize the accessibility of classes and members.

class Model_Member extends App_Model_Model

The class name is weird a little bit. Model of a model? App also too generic, something more descriptive would be better.

$member->CreatedOn = date('Y-m-d H:i:s');

Wouldn't be better using DateTime objects here?


Generating an array of unique values of a specific property in an object array Given a table represented as a javascript array of objects, I would like create a list of unique values for a specific property.

var table = [
  {
    a:1,
    b:2
  },
  {
    a:2,
    b:3
  },
  {
    a:1,
    b:4
  }
];

var groups = _.groupBy(table, "a");
var array = [];
_.forOwn(groups, function(value, key){
  array.push(key);
});

console.log(array);
<script src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/3.10.1/lodash.min.js"></script>
Check your console

The problems with this code is that the groupBy keep track of the values, which I really don't care about, and I need to iterate over the entire list generated by group again so I can yank out the keys. It seems pretty inefficient to me and Im wondering if there is a smarter way to do this without reinventing the wheel (or perhaps if I should just bite the bullet and reinvent the wheel).

I know this is an old post but purely for those looking for ways to do this heres a way that I found works nicely with a single line

var table = [
  {
    a:1,
    b:2
  },
  {
    a:2,
    b:3
  },
  {
    a:1,
    b:4
  }
];

let result = [...new Set(table.map(item => item.a))];
document.write(JSON.stringify(result));


Finding maxDate for each entity I have a table with (CompanyNumber, File_Date, [various financial results])
I wanted to get only the most recent set of accounts for each company.

After various googling I eventually got a query that does what I want it to do.

Is this a good way of doing it?

N.B. the File_Date field is actually a text field, so needs converting to Date before comparisons. Specifically, text --> varchar --> date.

select * from uk_registered_companies.dbo.Filed_Accounts
JOIN
(
Select companyNumber, max(convert(date, CONVERT(varchar(20),file_date),103)) as MaxDate from uk_registered_companies.dbo.Filed_Accounts
group by CompanyNumber
) mostRecentAccounts
on
Filed_Accounts.CompanyNumber = mostRecentAccounts.CompanyNumber
and
(convert(date, CONVERT(varchar(20),Filed_Accounts.File_Date),103)) = mostRecentAccounts.MaxDate

Quick review, there are a few things I think you could improve, they are mostly a matter of style though.

Casing

Just pick a casing style and stick to it, you are actually using all 3 casing styles kind of randomly.

Common Table Expression (or, CTE)

In Transact-SQL/ Microsoft SQL Server, there's a syntax sugar type of expression to help reduce sub-queries, the execution plan from the query engine should be the same but they help readability.

For instance, look at this join clause:

from
  uk_registered_companies.dbo.Filed_Accounts
JOIN (
  Select
      companyNumber,
      max(convert(date, CONVERT(varchar(20),file_date),103)) as MaxDate
  from
      uk_registered_companies.dbo.Filed_Accounts
    group by CompanyNumber
) as mostRecentAccounts

You could do something like this:

--Here is the CTE:
with mostRecentAccounts as (
        select
            companyNumber,
            max(convert(date, CONVERT(varchar(20),file_date),103)) as MaxDate
        from
            uk_registered_companies.dbo.Filed_Accounts
        group by CompanyNumber
)
--end of CTE
select
    *
from
    uk_registered_companies.dbo.Filed_Accounts
join
    mostRecentAccounts
  on
    Filed_Accounts.CompanyNumber = mostRecentAccounts.CompanyNumber
  and
    (convert(date, CONVERT(varchar(20),Filed_Accounts.File_Date),103)) = mostRecentAccounts.MaxDate

Indentation

I did a few changes to your formatting to make it easier to read. For indenting, I usually have the following keywords to the left, and indent everything else accordingly, kind of a crapshoot as there is no official style guide:

with Foo as (
    --...
)
select
  --...
from
  --...
join
  --...
    on --...
where
  --...
group by
  --...
having
 --...
order by
 --...


Simple web service call with retries This code is actually pretty basic. Basically, I'm doing a large number of web calls. I currently do up to 3 Web Service calls simultaneously, one of which is below.

There were 3 errors I saw in testing: Bad Gateway, Gateway Timeout, and basically "Couldn't establish a secure connection" (not the exact error text). In each case, a single retry appears to have fixed the issue.

"Bad Gateway" and "Gateway Timeout" didn't result in an exception, but "Couldn't establish a secure connection" did.

If I get either of the first two errors, it'll wait 2 seconds and try again. If it still didn't work, it tries 7 seconds and tries again. After that, it "gives up" and just returns null. (I've never seen that happen in testing, and I've never even gotten to the second retry). The 2-second and 7-second delays are somewhat arbitrary.

The code is below:

public static async Task<Job> GetJob(string id)
{
    int attempts = 1;

    tryAgain:

    string jobJson;

    try
    {
        using (HttpClient client = HttpClientConstructor.GetHttpClient(false, "application/json"))
        {
            HttpResponseMessage response = await client.GetAsync("jobs/" + id);

            if (!response.IsSuccessStatusCode)
            {
                switch (response.StatusCode)
                {
                    case HttpStatusCode.BadGateway:
                    case HttpStatusCode.GatewayTimeout:
                        await Task.Delay(2000);
                        response = await client.GetAsync("jobs/" + id);

                        if (response.StatusCode == HttpStatusCode.BadGateway || response.StatusCode == HttpStatusCode.GatewayTimeout)
                        {
                            await Task.Delay(7000);
                            response = await client.GetAsync("jobs/" + id);
                        }
                        break;
                }

                if (!response.IsSuccessStatusCode)
                {
                    string errorMessage = "Failed I have. Reason: " + response.ReasonPhrase;
                    Console.WriteLine(errorMessage);

                    System.Diagnostics.Trace.TraceError(errorMessage);

                    // If multiple retries couldn't get it, just give up
                    return null;
                }
            }

            jobJson = await response.Content.ReadAsStringAsync();
        }

        Job job = JsonConvert.DeserializeObject<Job>(jobJson);

        return job;
    }
    catch (Exception e)
    {
        Console.WriteLine(e.ToString());

        System.Diagnostics.Trace.TraceError(e.ToString());

        // Go for two retries
        // This actually worked in my testing
        // In the rare case where I got exceptions, a retry worked
        if (attempts <= 3)
        {
            attempts++;

            // Is goto evil here?
            goto tryAgain;
        }

        return null;
    }
}

Obviously, this will be running asynchronously. As mentioned above, I will run up to 3 calls at once. It's fixed at 3 to avoid being "throttled" by rate limits on the API.

Is this a good way to solve this?

Could this code be improved?

In general if you have a goto statement, you should reorder your code to not need it. In this case, your code actually tries 9 times, not three, because you await client.GetAsync 3 times, and then jump back to your goto label up to 3 times.

The simple fix in this case is to remove the extra response=await... calls, enclose the entire try/except block in a loop and break or return if the call is successful. Something like:

for (int i = 0; i < 3; i++){
    try
    {
        HttpResponseMessage response = await client.GetAsync("jobs/" + id);
    }
    catch (CantConnectExcept as ex){
        await Task.Delay(2000);
        continue;
    }

    if (!response.IsSuccessStatusCode){
        await Task.Delay(2000);
        continue;
    }

    jobJson = await response.Content.ReadAsStringAsync();
    return JsonConvert.DeserializeObject<Job>(jobJson);
}
return null

It doesn't sound like waiting more seconds on later failures is important. If you find it is important, you can do something like:

List<int> waitTime = new List<int> {2000, 2000, 7000};
...
await Task.Delay(waitTime[i]);


Retrieve all parents, all possible children, sorted and with their "depth" I have a model section with a column parent_id - the only thing identifying a parent/child relationship. I wrote a helper method to output all sections, all their possible subsections and include their "depth".

This is the helper method:

# Calls itself for each section recursively to fetch all possible children
def fetch_all_sections(standard, section = nil, depth = 0)
  all_sections = []
  if section.nil?
    rootsections = roots(standard.sections.sorted)
    if ! rootsections.nil?
      rootsections.each do |section|
        all_sections.push(fetch_all_sections(standard, section))
      end
    end
  else
    all_sections.push({:id => section.id, :sortlabel => section.sortlabel, :title => section.title, :depth => depth})
    section.children.sorted.each_with_index do |section, index|
      all_sections.push(fetch_all_sections(standard, section, (depth += 1) - index))
    end
  end
  return all_sections
end

sorted is a module method which sorts based on the sort_key column using regular expressions. The output of debug(fetch_all_sections(@standard)) is:

- - "1 Collaborase Test Document depth: 0"
  - - "1.1 Section One Dot One depth: 1"
  - - "1.a Testing 123 depth: 1"
  - - "1.b Section One dot B depth: 1"
    - - "1.b.1 Testing One B One depth: 2"
- - "2 Identify Project GHG Sources, Sinks, and Reservoirs (SSRs) depth: 0"
  - - "2.a Section Two dot A depth: 1"
- - "3 Identify the Baseline depth: 0"
- - "4 Identify the Baseline GHGs depth: 0"
- - "5 Identify SSRs Relevant for Quantification depth: 0"
  - - "5.1 Section Five Dot One depth: 1"
- - "6 GHG Quantification depth: 0"
- - "7 Monitoring depth: 0"
- - "8 Data and Information Retention depth: 0"
- - "9 QA/QC depth: 0"

My questions is this:

In this loop:

<% fetch_all_sections(@standard).each do |section| %>
  <%= debug section %>
<% end %>

I want each section and all their children to be separated. Instead, each section in the above loop contains their children.

Instead of this:

---
- - :id: 122
    :sortlabel: "5"
    :title: Identify SSRs Relevant for Quantification
    :depth: 0
- - - :id: 412
      :sortlabel: "5.1"
      :title: Section Five Dot One
      :depth: 1

I want this:

---
- - :id: 122
    :sortlabel: "5"
    :title: Identify SSRs Relevant for Quantification
    :depth: 0
---
- - :id: 412
    :sortlabel: "5.1"
    :title: Section Five Dot One
      :depth: 1

What I needed was the pipe operator! It was of course just doing what I was telling it to - pushing the results of itself onto the current element in the array. Only when the rootsections loop completes does it start a new array element.

The 'pipe' (|) is a different array merge operator which appends what's to the right of it onto the end of the array on the left.

all_sections | fetch_all_sections(standard, section, depth)

The pipe operator changed how depth worked so instead I just counted the number of "." in sortlabel:

depth = section.sortlabel.split('.').length -1

The full method:

# Calls itself for each section recursively to fetch all possible children
def fetch_all_sections(standard, section = nil, depth = 0)
  all_sections = []

  if section.nil?
    rootsections = standard.sections.sorted
    if ! rootsections.nil?
      rootsections.each_with_index do |section, i|
        depth = section.sortlabel.split('.').length - 1
        all_sections << fetch_all_sections(standard, section, depth).first
      end
    end
  else

    all_sections << {:id => section.id, :sortlabel => section.sortlabel, :title => section.title, :depth => depth}

    section.children.sorted.each do |section|
      all_sections | fetch_all_sections(standard, section)
    end
  end

  return all_sections
end

If someone seeing this is considering something similar but starting from scratch, consider an Active Record Nesting gem: https://www.ruby-toolbox.com/categories/Active_Record_Nesting

UPDATE: I added .first within the rootsections loop. This is because I inadvertently built an array of arrays, each containing 1 hash. What I wanted was an array of hashes. Much easier retrieval now!


Securely connecting to my DB Hello all I have a short question. Is this a good and secure way to connect to my DB ?
session_start();

$servername = "localhost";
$un = "username";
$password = "password";
$dbname = "project";

$timezone = date_default_timezone_set("America/New_York");

// Create connection
$con = mysqli_connect($servername, $un, $password, $dbname);

if($con == true) {
}

// Check connection
if ($con->connect_error) {
    die("Connection failed: " . $conn->connect_error);
}

I agree with Anonymous that there isn't much to review here.
I would like to point out a few general things.
The db connection credentials are the type of thing that I'd put in a config.php file and include it where necessary.
$timezone is probably going to be useless in the longterm.  date_default_timezone_set() returns a boolean value to inform you of whether or not you have supplied a valid timezone.  Basically, once this tested once, there is no need to check the return value ever again (until it is changed, of course).
$dbCredentials = [
    "localhost", //server name
    "username",
    "password",
    database"database"
];

date_default_timezone_set("America/New_York");

Then just include the file and connect by unpacking the indexed array of credentials like this:
$con = new mysqli(...$dbCredentials);

Notice this is object-oriented syntax.  You should not be mixing styles.
When you are in a development environment (not a production environment), you may return raw error messages. In production, you must never provide raw error messages to users.
When checking for a failed connection, a falsey check (no function call) will be sufficient.  It is vital that you spell your connection variable correctly.
if (!$con) {
    die("Connection failed: " . $con->connect_error);
}

That said, an overarching refinement is to perform error handling elsewhere. See YourCommonSense's advice on error reporting.
session_start(); is usually something that you would put at the top of an include that is run before anything else.


Options with a "yes" or "no" radio button I have about 6 of these, but I'm only posting two. Essentially I have a bunch of options with a "yes" or "no" radio button. If "no" is selected, I want a form to popup.

$('#Option1').live('change', function() {
    if ($('input[name=Option1]:checked').val() == "False") {
        $('#formDiv').show();
    } else {
        $('#formDiv').hide();
    }
});
$('#Option2').live('change', function() {
    if ($('input[name=Option2]:checked').val() == "False") {
        $('#formDiv').show();
    } else {
        $('#formDiv').hide();
    }
});

How can I make this more efficient, or even consolidate it? Or is having a function for each pair of radio buttons necessary?

This should work:

$('#Option1, #Option2').live('change', function() {
    if ($('input[name=' + $(this).attr('id') + ']:checked').val() == 'False') {
        $('#formDiv').show();
    else {
        $('#formDiv').hide();
    }

    });

Or, if whatever has the id option1 is the same page element as the input with the name option1, you could do:

$('#Option1, #Option2').live('change', function() {
    if ($(this).val() == 'False')
        $('#formDiv').show();
    else {
        $('#formDiv').hide();
    }

    });


Suggestions for improvement on Project Euler #7 Here is a solution for Project Euler Problem #7.

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

I've used the pseudo code from the Wikipedia page for the Sieve of Eratosthenes.

  Input: an integer n > 1

  Let A be an array of Boolean values, indexed by integers 2 to n, initially all set to true.

for i = 2, 3, 4, ..., not exceeding √n:
     if A[i] is true:
         for j = i^2, i^2+i, i^2+2i, ..., not exceeding n :
             A[j] := false

Any suggestions would be much appreciated.

import java.util.*;

public class FindPrimesHashMap {
    public static void getPrime(int x) {
        int n = (x * ((int)Math.sqrt(x) / 10)) + x;
        int nSqrt = (int)Math.floor(Math.sqrt(n));
        int j = 0;
        int k = 0;
        Map<Integer, Boolean> primeMap = new HashMap<Integer, Boolean>();
        Set<Map.Entry<Integer, Boolean>> primes = primeMap.entrySet();
        ArrayList<Integer> primeArray = new ArrayList<Integer>();

        for (int a = 2; a < n; a++) {
            primeMap.put(a, true);
        }

        for (int i = 2; i <= nSqrt; i++) {
            if (primeMap.get(i)) {
                while (true) {
                    j = (i * i) + (k * i);
                    k++;

                    if (j > n) {
                        break;
                    }
                    primeMap.put(j, false);
                }
                j = 0;
                k = 0;
             }
        }

        for (Map.Entry<Integer, Boolean> entry : primes) {
            if (entry.getValue()) {
                primeArray.add(entry.getKey());
            }
        }
        Collections.sort(primeArray);
        System.out.println(primeArray.get(x - 1));
}

You have a proliferation of cryptically named variables, making your code hard to follow.

Variables j and k are only ever used inside the while (true) loop, so they should be declared in a tighter scope.  There is no need to reset j after the loop.  Also, it would be easier to understand your code if you reset k = 0 before the loop rather than afterwards.  The while (true) loop could be simplified to:

for (int k = 0; ; k++) {
    int j = (i * i) + (k * i);
    if (j > n) {
        break;
    }
    primeMap.put(j, false);
}

Then, why not eliminate k altogether?

for (int j = i * i; j <= n; j += i) {
    primeMap.put(j, false);
}

I don't see why you set n = x ⌈√x / 10⌉.  It seems that you chose it as an estimate for the upper bound of your answer, though I don't understand the justification.  The answer should be somewhere around px ≈ x  ln(x).

In the end, it's not really important how you arrived at n, as long as it is large enough.  An explanatory comment would have been appreciated, though.

Using a HashMap to implement the Sieve of Eratosthenes is not a great idea, because:

You want to iterate through the results consecutively
All of the keys were initially consecutive
Internally, it will be hashing each key to another number

It would be a lot simpler if you simply used an array of booleans.


Simple image loader using SDL and C++ I have been coding Java for 5+ years and before that did a little professional C. Picking up C++ again for the fun of it and I feel like I am just writing Java in C++ rather than understanding the C++ way of doing things.

Anyway, I wanted some thoughts on my simple little class to make sure I am C++ing right.

This is my header:

#pragma once

#include <iostream>

#include <SDL.h>
#include <SDL_image.h>

class ImageResource final
{
    SDL_Surface* surface;

public:
    ImageResource(std::string& path);
    ~ImageResource();

    SDL_Surface* get_surface();
};

This is the implementation:

#include "ImageResource.h"
#include <SDL.h>
#include <SDL_image.h>

ImageResource::ImageResource(std::string& path)
{
    this->surface = IMG_Load(path.c_str());
    if (this->surface == NULL)
    {
        throw std::runtime_error{ std::string{"Unable to load image: "} + path };
    }
}

ImageResource::~ImageResource()
{
    SDL_FreeSurface(this->surface);
}

SDL_Surface * ImageResource::get_surface()
{
    return this->surface;
}

Things I don't know if I am doing right are const correctness (actually I am pretty sure I am doing it wrong because nothing is const), exception throwing, and general style. Any other comments are welcome.

Overall, it looks pretty good. Not very much code, so not very much that could be bad. I'll give you some tips, from my perspective:

This first note isn't exactly about C++, but more common than that: It's a good idea to put "public" things first in your class, and more protected things further down. This is because when people look at the class, they are more likely looking for public things, rather than protected/private things. So if you put those at the top, they don't have to scroll through to find them.

Consider making get_surface(); an inlined function, by defining it directly in the header. It's a shame if such a quick and simple function would incur the cost of a function call, and the cost of inlining such a tiny thing is near to nothing.
As I see it, all direct/simple getters and setters should be inlined.

There is no need to write this->, since that is always implied. They only clutter the code.

It is good practice to point out which variables are actually protected members, in the name of the member. I personally put a small "m" in front of all protected members, such as "mSurface". Some people write "m_Surface", or just a "_" in front, with no "m", and so on.

The part that says (this->surface == NULL), you should use nullptr instead of NULL. It's a keyword from C++11, assuming you're using at least C++11. nullptr is a pointer-type, while NULL is simply an int. Writing NULLis exactly the same as writing 0. In some cases, this causes ambiguity, so it's good practice to always use this keyword for null-pointers, since it sometimes actually matters, even if not always.

And as for const-correctness, there is only one function to consider, here. And since it returns a pointer, which could be used to change internal data, it shouldn't really be considered a const member function. (Or "method", to you Java people.)