Interactive command-line binary to decimal converter Is this the most efficient way to write this piece of code? Is there a better way of handling errors inside of a while/for loop, instead of calling back to the original function? I have tried using break or continue, but it breaks the functionality of my program. Any help would be awesome, thanks.

## calculate binary to decimal ##

def binaryToDecimal(binary):
    ### reverse the number
    decimal_num = 0
    reverse_binary = binary[::-1]
    for x in range(7,-1,-1):
        if len(binary) <= x:
            continue
        if int(reverse_binary[x]) == 1:
            decimal_num += 2**x
    print(f"Your binary number converts to {str(decimal_num)}. ")
    binaryToDecimal(checkUI())

def getUserInput():
    return input("Please enter a binary number that you "
                 "want to convert into a decimal.\n Type 'quit' to quit program.\n")

def checkUI():
    userInput = getUserInput()
    if userInput == "quit" or userInput == "q":
        quit()
    for character in userInput:
        try:
            if len(userInput) > 8:
                print("Binary numbers are a maximum of 8 digits. Please try again")
                binaryToDecimal(checkUI())
            val = int(character)
            if val != 1 or 0:
                print("Invalid binary number. Please enter a 1 or 0 (binary numbers: 11110000)")
                binaryToDecimal(checkUI())
        except ValueError:
            print("You did not enter a binary number. (11110000) Please try again.")
            binaryToDecimal(checkUI())

    return str(userInput)

binaryToDecimal(checkUI())

TL;DR:

It's  unlikely to beat Python's own implementation of this which is basically as simple as

decimal = int(binary, base=2)

in terms of convenience, clearity, and performance.
This will throw a ValueErrorin case the number cannot be converted to a binary, e.g. if the user enters something like 101010103.

Welcome to the world of Python, where "batteries included" is a real thing!

If you, for whatever reason, do not want to use the most convenient Python has to offer, it is still possible to write a Pythonic alternative:

decimal = sum(int(digit)* 2**index for index, digit in enumerate(user_input[::-1]))

This little piece of code is called generator expression (relevant PEP, good Stack Overflow post) and is basically the equivalent of:

decimal = 0
for exponent, digit in enumerate(user_input[::-1]):
    decimal += digit * 2**exponent

which is close(r) to what you did in your original implementation. Apart from that you don't have to care about the actual length of the input.

Detailed review

Of course there is more to say about your code than this, especially since you're asking about other aspects as well. So let's look at your code.

Style

Python has an official Style Guide called PEP8 (batteries included, you remember?). The main takeaways that are follow by the better part  of Python programmers are

Use snake_case for variable and function names and CamelCase/PascalCase for class names.
Write function documentation (often just docstrings) """Enclosed in triple quotes""".

I will demonstrate how to apply these principles to your code in a moment.
There are other useful recommendations which help you to keep the appearance of your code clean and readable, so it's definitely worth a read.

Naming

Apart from the stylistic point in the names, there is always the name itself. It's good that you decided to use functions and most of their names are totally fine. The odd one out is clearly checkUI. If you were just left with this name, I highly doubt that you would be able to tell what it does. Since it basically handles most of the interaction with the user, as well as validating and converting the numbers, the obvious (though maybe not ideal) name would be simply main().

The script itself

After we have gone through some high-level aspects of the code, let's dive into the details. I will go through the code function by function.

binaryToDecimal - soon to be binary_to_decimal

As I already said above, the core functionality of the function can basically be replaced by a single line of Python code. And that's likely all the function should do. Apart from the main function, all of them should basically have a single responsibility. At the moment there are three

converting the number
printing the converted number
starting a new round of user input

The 2nd one is easy to fix: simple move the call to print from binary_to_decimal to main. The 3rd one is really, really (and I mean really really) bad design. The conversion function should absolutely not be tasked with starting a new round of user input. To make it worse, this will also lead to recursive calls of your functions, that will eventually end with a RuntimeError when your program exceeds the maximum recursion depth allowed by the Python interpreter. So drop that and never do it again!

getUserInput - soon to be get_user_input

The function is basically fine and a prime example of single responsibility. Maybe it's functionality is even a bit to minimalistic, so you might not actually need it.

checkUI - soon to be main

This is where things get serious. The first and foremost recommendation for this piece of code is similar to the first function we looked at: Get rid of the recursion!. Recursion as such is not a bad thing and valid programming technique in their own right, but not for this purpose!

The idiomatic way to ask for user input repeatedly would be something like

def main():
    """Repeatedly ask for user input and convert its decimal equivalent"""
    while True:
        # read and normalize the user input
        # Note: this might also (better) be done in get_user_input
        user_input = get_user_input().strip().lower()
        if user_input.startswith("q")
            break

        # further validation and conversion
        ...

if __name__ == "__main__":
    main()

The check to stop the program was slightly modified to be a little bit more robust. The way I chose to implement it will also make sure that things like Q, Quit, qsomethingelse, and the like end the program. If that's not what you want, simply replace it with the check you had originally. I also added if __name__ == "__main__": which adds as a kind of safeguard to make sure the "interactive" of your code is only triggered if you use it as script and not try to import it.

Now for the validation and conversion part. With all the other changes in place it will become as simple as

def main():
    """Repeatedly ask for user input and convert its decimal equivalent"""
    while True:
        # read and normalize the user input
        # Note: this might also (better) be done in get_user_input
        user_input = get_user_input().strip().lower()
        if user_input.startswith("q"):
            break

        if len(user_input) > 8:
            print("Binary numbers are a maximum of 8 digits. Please try again")
            continue

        try:
            decimal = binary_to_decimal(user_input)
            print(f"Your binary number converts to {decimal}. ")
        except ValueError:
            print("You did not enter a binary number. (11110000) Please try again.")

Since you chose to limit binary numbers to a maximum length of 8bit (why?), you should likely also include this in the prompt.


2-opt algorithm for traveling salesman I wrote a 2-opt algorithm to be used in a program and noticed (using profile) that the 2-opt is eating up a lot of time. I have tried a few things to make it run faster, but I am out of ideas. Any tips on making it run faster, or a better way to write a 2-opt algo?

def twoOpt(self):
    '''Peforms 2-opt search to improve route'''
    for index, route in enumerate(self.routes):
        for element in xrange(len(route.fields)):
            i = element + 1
            k = i + 1
            while k <= len(route.fields):
                newRoute = Route(route.scout)
                for field in self.twoOptSwap(route.fields, i, k):
                    newRoute.addField(field)
                newRoute.calcRouteCost()
                if newRoute.cost < route.cost:
                    self.routes[index] = newRoute
                k += 1

def twoOptSwap(self, fields, i, k):
    start = fields[0:i]
    middle = fields[i:k]
    middle = middle[::-1]
    end = fields[k:]
    newFields = start + middle + end
    return newFields

Each route object contains a list of fields (route.fields) and route.calcRouteCost() sets route.cost with a value.

A slice of a list is a new list, so this code constructs five lists:

start = fields[0:i]
middle = fields[i:k]
middle = middle[::-1]
end = fields[k:]
newFields = start + middle + end

This is probably faster:

newFields = fields[:]
newFields[i:k] = fields[k-1:i-1:-1]

Instead of adding fields one by one in the code below, you could change Route to accept the list of fields in the constructor.

newRoute = Route(route.scout)
for field in self.twoOptSwap(route.fields, i, k):
    newRoute.addField(field)

Note also that after reversing a segment of the route, a complete recalculation of cost is not necessary. You only need to consider how the cost changes where the segments join, assuming cost does not depend on direction of travel.
Instead of

for element in xrange(len(route.fields)):
    i = element + 1

simply

for i in xrange(1, len(route.fields) + 1):

Instead of a while loop

k = i + 1
while k <= len(route.fields):
    ...
    k += 1

a for loop

for k in xrange(i + 1, len(route.fields) + 1):


Flask route to pull data from database for getJSON to process I am developing a reporting application in Flask where I pull data in a route function from a postgresql database and jsonify the data. On the client side I have some javascript that does a $.getJSON on the route to request the data.

Here is what the code looks like.

Flask route

def get_db():
    if not hasattr(g, 'postgres_db'):
        g.postgres_db = connect_db()
    return g.postgres_db

@main.route('/Q5')
@login_required
def failed_access():
    failed_query = """select * from prototype.Q5"""
    db = get_db()
    with db.cursor(cursor_factory=RealDictCursor) as cur:
        cur.execute(failed_query)
        results = cur.fetch_all()
     data = json.dumps(results, indent=2, default=date_handler)
     return data

Client side javascript looks like this:

$.getJSON('/Q5', function genChart() { blah() }));

Everything works fine but I want to make sure that there is something I am not missing or could be doing a better way.

I don't really get the purpose of the get_db function. Why not simply initialize the database at the very beginning like so:

db = connect_db()

And then use db in failed_access directly. It seems you wanted to do lazy initialization, but is it really worth it? Sooner or later, the database will be used anyway. Initializing upfront is straightforward, simple, and less error prone.


Module for building swing tables I have a module that builds swing tables based on an Object[][], then I have another module that queries an SQLite database and returns a ResultSet.

I have written a method that converts the ResultSet to an ArrayList<Object[]> first, then further into an Object[][]. The reason I do it this way is because you cannot get the row count from the ResultSet without iterating over it.

I'm wondering if there is a more efficient way to build my Object[][] from the ResultSet without having to iterate over the data twice.

Here is the method that performs the conversion:

public Object[][] executeQuery(String query) throws SQLException{
    ResultSet rs = getResultSet(query);
    ResultSetMetaData rsMetaData = rs.getMetaData();
    int columnCount = rsMetaData.getColumnCount();
    ArrayList<Object[]> result = new ArrayList<Object[]>();
    Object[] header = new Object[columnCount];
    for (int i=1; i <= columnCount; ++i){
        Object label = rsMetaData.getColumnLabel(i);
        header[i-1] = label;
    }
    while (rs.next()){
        Object[] str = new Object[columnCount];
        for (int i=1; i <= columnCount; ++i){
            Object obj = rs.getObject(i);
            str[i-1] = obj;
        }
        result.add(str);
    }
    int resultLength = result.size();
    Object[][] finalResult = new Object[resultLength][columnCount];
    finalResult[0] = header;
    for(int i=1;i<resultLength;++i){
        Object[] row = result.get(i);
        finalResult[i] = row;
    }
    return finalResult;
}

Well, you could just call the toArray method of your list to transform the list into an array. Or even better, you could make the table model use a List<Object[]> rather than an Object[][], which would avoid the unnecessary conversion.


Menu class/struct for RPG I plan to use this menu struct in an RPG played in the terminal.  I want to make a RPG similar to D&D. I would like advice on my methods for the menu struct, especially the "hide" and "unhide" option methods.

main.cpp

#include <string>
#include <vector>
#include <cstdlib>
#include <iostream>
#include <initializer_list>

const int invalid_index = -2;
const int default_index = -1;
const std::string defualt_title = "";
const std::initializer_list<std::string> defualt_options = {""};

struct menu{
    menu(const std::string& title, const std::initializer_list<std::string>& options)
        : title(title), options(options), current_index(default_index){;};
    menu(void)
        : title(defualt_title), options(defualt_options), current_index(default_index){;};

    std::vector<std::string> hidden_options;
    std::vector<std::string> options;
    std::string title;
    int current_index;

    void insert_option(int index, std::string option){
        if(index >= 0 && index < options.size()){
            options.insert(options.begin() + index, option);
        }else{;}
    };

    void delete_option(int index){
        if(index >= 0 && index < options.size()){
            options.erase(options.begin() + index);
        }else{;}
    };

    void obtain_option(int index){
        if(index >= 0 && index < options.size()){
            current_index = index;
        }else{current_index = invalid_index;}
    };

    void hide_option(int index){
        if(index >= 0 && index < options.size()){
            hidden_options.resize(options.size());
            hidden_options[index] = options[index];
            options.erase(options.begin() + index);
        }else{;}
    };

    void unhide_option(int index){
        if(hidden_options[index] != ""){
            if(index >= 0 && index < options.size()){
                options.insert(options.begin() + index, hidden_options[index]);
            }else{;}
        }else{;}
    };

    friend std::ostream& operator<<(std::ostream& ostream, const menu& r){
        ostream << r.title << '\n';
        for(int i = 0; i < r.options.size(); i++){
            ostream << "[" << i << "] " << r.options[i] << '\n';
        }ostream << "Enter index: ";
        return ostream;
    };
};

int main(){
    menu title_screen("Anyone Know a good RPG?", {"Play","Help","Quit"});
    return 0;
}

defualt_title and defualt_options are misspelled.
Those empty else{;} statements are unnecessary.  If there is eventually a need for actual else statements, then they can be added later.
With those "stray" data members, this should really be a class so that they can be private.  Data members should not be directly exposed to the interface.  Any functions that shouldn't be called by the user should also be private, otherwise they should be public.


TapeEquilibrium Codility implementation not achieving 100% Given the following task description from here :

  A non-empty zero-indexed array A consisting of N integers is given.
  Array A represents numbers on a tape.

  Any integer P, such that 0 < P < N, splits this tape into two
  non-empty parts: A[0], A[1], ..., A[P − 1] and A[P], A[P + 1], ..., A[N − 1].

  The difference between the two parts is the value of: |(A[0] + A[1] + ... + A[P − 1]) − (A[P] + A[P + 1] + ... + A[N − 1])|

  In other words, it is the absolute difference between the sum of the
  first part and the sum of the second part.

  For example, consider array A such that:

  A[0] = 3
  A[1] = 1
  A[2] = 2
  A[3] = 4
  A[4] = 3

  We can split this tape in four places:

P = 1, difference = |3 − 10| = 7
P = 2, difference = |4 − 9| = 5
P = 3, difference = |6 − 7| = 1
P = 4, difference = |10 − 3| = 7

  Write a function:

def solution(A)

  that, given a non-empty zero-indexed array A of N integers, returns
  the minimal difference that can be achieved.

  For example, given:

  A[0] = 3
  A[1] = 1
  A[2] = 2
  A[3] = 4
  A[4] = 3

  the function should return 1, as explained above.

  Assume that:

  N is an integer within the range [2..100,000]; each element of array A
  is an integer within the range [−1,000..1,000]. Complexity:

  expected worst-case time complexity is O(N);
  expected worst-case space complexity is O(N), beyond input storage (not counting the storage required for input arguments).

  Elements of input arrays can be modified.

I decided to implement an algorithm that with 2 counters/pointers (one from leftmost and another one from rightmost of the array input) representing the total sum of values that the pointer(s) have traversed.
The process works by first deciding which pointer to move closer to the other in each iteration, which is looking the next element directly next to the location of the current pointer, attempt to temporarily sum the value to the pointer, and then find the absolute difference between the other pointer.
The absolute difference is also calculated for the other pointer, and then compared against each other's pointer temporarily accumulated value, to find out which one yields lower absolute difference.
The pointer move that yields the lowest absolute difference then performs the actual summation to the pointer, and that particular pointer moves for that iteration.

The following is my code :

from math import fabs

def solution(A):
    l_ptr = 0
    r_ptr = A.__len__() - 1
    l_sum = A[l_ptr]
    r_sum = A[r_ptr]
    while l_ptr < r_ptr - 1:
        if fabs(l_sum + A[l_ptr + 1] - r_sum) > fabs(l_sum - (r_sum + A[r_ptr - 1])):
            r_ptr -= 1
            r_sum += A[r_ptr]
        else:
            l_ptr += 1
            l_sum += A[l_ptr]
    return (int)(fabs(l_sum - r_sum))

In the test case, I didn't manage to achieve 100% test accuracy, and I'm not sure exactly why, but I think perhaps it has something to do with the pointer not being able to look for the array element that are several steps away at a given iteration and the possibility of having negative value.
The followings are the test cases that it fails on according to Codility :

▶ small_random
random small, length = 100 ✘WRONG ANSWER
got 269 expected 39
▶ large_ones
large sequence, numbers from -1 to 1, length = ~100,000 ✘WRONG ANSWER
got 228 expected 0
▶ large_random
random large, length = ~100,000 ✘WRONG ANSWER
got 202635 expected 1

Obviously provided that it's evaluation of a test set, the actual input data for the test are not given, so it's more difficult for me to figure out what part of my algorithm that is incorrect and the causes.
Could someone provide an explanation (preferably with a sample input data) what I misunderstood and the cause?
Thank you in advance.

Algorithm

Your algorithm works for input with only positive integers.
But it may not work with some input that contains negative numbers,
for example it gives incorrect result for:

[-1, 1, 1, -1, -2]

Why does it work for all positive numbers?

At any point in your loop, you basically have:

leftsum: the sum of elements on the left so far
leftnext: the next element on the left
rightsum: the sum of elements on the right so far
rightnext: the next element on the right

When you know that all remaining elements in the middle are non-negative,
then you can safely decide whether to take the left or the right,
by minimizing the difference between leftsum + leftnext and rightsum + rightnext.
This is safe, because all the remaining elements are non-negative,
therefore the difference can only shrink, or otherwise be minimal.

But when there can be negative numbers in the middle,
you don't have such knowledge,
and it can be impossible to decide which side to advance.

Consider this alternative that's simple and intuitively easy to understand,
and it's guaranteed to give correct result:

Set left to the first element
Set right to the total sum - left
Initialize mindiff to the absolute difference of left and right
Iterate from the 2nd element until the -1th: add to left and subtract from right and update mindiff (A quick tip for writing the loop: for value in A[1:-1]: ...)
return mindiff

Technique

It's strange you did from math import fabs when you don't need floating point math to solve this problem. You can use abs instead of fabs.

Instead of A.__len__() it's more natural to use len(A).


Find the most frequent number in an array I implemented a solution for: Finding the most frequent number from an array. How are my variable names? Or how could I improve this?

 private static Integer findMostFrequentNumber(int[] givenArray) {
    int mostFrequentNumber = 0;
    if (givenArray != null) {
        Map<Integer, Integer> numberOccuranceMap = new HashMap<>();
        int mostFrequentOccurance = 0;

        for (Integer number : givenArray) {
            if (!numberOccuranceMap.containsKey(number)) {
                numberOccuranceMap.put(number, 1);
            } else {
                numberOccuranceMap.put(number, numberOccuranceMap.get(number) + 1);
            }
            if (numberOccuranceMap.get(number) > mostFrequentOccurance) {
                mostFrequentNumber = number;
                mostFrequentOccurance = numberOccuranceMap.get(number);
            }
        }
    }
    return mostFrequentNumber;
}

Please add a /** */ javadoc sentence, clarifying what happens in e.g. the case where { 5, 5, 6, 6 } is passed in.

The name findMostFrequentNumber() seems a bit verbose, when mode() or findMode() would suffice.

Consider numbers instead of givenArray. In any event, "array" is redundant with int[].

Consider eliding the null check. Caller gets what he deserves if the contract is "caller shall provide an array of numbers". NPE is appropriate if caller fails to do that. Or throw your own exception. Returning zero is clearly inappropriate, as caller cannot distinguish between an input of null and an input of { 5, 6, 0, 0, 0, 5 }.

In numberOccuranceMap, the "map" is redundant with the Map declaration. (Also, typo: occurrence.) Consider a "to" name for your maps, such as numToOccurences or numToCount.

    int mostFrequentOccurance = 0;

This sounds like it is simply modeCount, and the mostFrequentNumber we will return is simply mode.

            numberOccuranceMap.put(number, numberOccuranceMap.get(number) + 1);

Your get() call could specify a 2nd argument default value of zero, which would obviate the need for the containsKey() test.

        if (numberOccuranceMap.get(number) > mostFrequentOccurance) {
            mostFrequentNumber = number;
            mostFrequentOccurance = numberOccuranceMap.get(number);
        }

Here we find the answer to the javadoc question above: last one wins!


PHP login redirect security I wish to redirect users to the login page if they attempt to visit a page which requires them to be logged in. After logging in, however, I want to redirect the user back to their original destination. I've written a "redirect.php" script which is to be included on all such pages:

<?php
require "session.php";
if(!$user){
    header("Location: login.php?dest=".urlencode($_SERVER["REQUEST_URI"]));
    die();
}
?>

Then on my login page I have the following:

<?php
$dest = "./";
if(isset($_GET["dest"])){
    $dest = $_GET["dest"];
}
?>

with the following JavaScript:

var URL = "<?php echo $dest; ?>";
//...
//upon successful login (via AJAX):
window.location.replace(URL);

Everything here works as intended but where does this stand from a security standpoint?

One vulnerability that comes to mind is something like

  http://mysite.com/login.php?dest=http://phishingsite.com

How might I best prevent something like this? Would regex be suitable here?

Are there any other security concerns with this type of thing? Perhaps a standard way of doing this? Or better yet, a method which does not use GET variables at all?

The way you're doing it has advantages over a session based redirect because you have the ability to link people directly to that login with redirect URL.

The disadvantage is phishing as you pointed out.

http://www.mysite.com/login.php?dest=http://mysite.phishing.com/

The way you're doing this, using $_SERVER['REQUEST_URI'] means you expect the $_GET['dest'] to only include a path, not a domain.

So you could add a check in the login page where it sets the $dest

$dest = "./";
if(isset($_GET["dest"]))
{
    $parts = parse_url( $_GET['dest'] );

    // if there is no `host` key set in the parse_url array, then its an internal path
    // so set the new $dest
    if( !isset( $parts['host'] ) )
    {
        $dest = $_GET["dest"];
    }
}


LeetCode: Linked List Cycle Challenge

https://leetcode.com/explore/interview/card/top-interview-questions-easy/93/linked-list/773/

  Given a linked list, determine if it has a cycle in it.

  To represent a cycle in the given linked list, we use an integer pos
  which represents the position (0-indexed) in the linked list where
  tail connects to. If pos is -1, then there is no cycle in the linked
  list.

  Example 1:

  Input: head = [3,2,0,-4], pos = 1 Output: true Explanation: There is a
  cycle in the linked list, where tail connects to the second node.

  Example 2:

  Input: head = [1,2], pos = 0 Output: true Explanation: There is a
  cycle in the linked list, where tail connects to the first node.

  Example 3:

  Input: head = [1], pos = -1 Output: false Explanation: There is no
  cycle in the linked list.

Follow up

Can you solve it using \$O(1)\$ (i.e. constant) memory?
Can you please review about performance?

using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace LinkedListQuestions
{
 public class ListNode
    {
        public int val;
        public ListNode next;

        public ListNode(int x)
        {
            val = x;
        }
    }

    [TestClass]
    public class HasCycleTest
    {
        [TestMethod]
        public void HasCycle()
        {
            ListNode head = new ListNode(3);
            head.next = new ListNode(2);
            head.next.next = new ListNode(0);
            head.next.next.next = new ListNode(-4);
            head.next.next.next.next = head.next; //2
            Assert.IsTrue(HasCycleClass.HasCycle(head));
        }

        [TestMethod] public void NoCycle()
        {
            ListNode head = new ListNode(3);
            head.next = new ListNode(2);
            head.next.next = new ListNode(0);
            Assert.IsFalse(HasCycleClass.HasCycle(head));
        }

        [TestMethod]
        public void OneItem()
        {
            ListNode head = new ListNode(3);
            Assert.IsFalse(HasCycleClass.HasCycle(head));
        }
    }

    public class HasCycleClass
    {
        public static bool HasCycle(ListNode head)
        {
            if (head == null || head.next == null)
            {
                return false;
            }
            ListNode slow = head.next;
            ListNode fast = head.next.next;
            while(fast!= null)
            {
                if (fast == slow)
                {
                    return true;
                }
                else
                {
                    slow = slow.next;
                    if (fast.next == null)
                    {
                        return false;
                    }
                    fast = fast.next.next;
                }
            }

            return false;
        }
    }
}

Performance

You have implemented Floyd’s Cycle-Finding Algorithm which adheres to \$0(1)\$ storage space. An alternative exists Brent’s Cycle Detection Algorithm which uses the same storage space. Check out this review on Computer Science SE for a comparison. It appears in general, Brent's algorithm is faster.

  According to Brent's paper, the complexity of Floyd's algorithm is between 3max(m,n) and 3(m+n), and that of Brent's is at most
  2max(m,n)+n, which is always better than 3max(m,n).

courtesy of Yuval Filmus' answer at CS

Style Guidelines

use var to declare a variable, specially when the type can be inferred from code
use a separate line for declaring attributes on top of members
use a white space after a method name and the opening parenthesis
use a white space after the while statement
use white space around operators (!=)
remove redundant nested else branches if the if branch always performs a return


Get a specific value from a big JSON response I have this code to access travel time from the Google directions api JSON result:

let jsonResult = try JSONSerialization.jsonObject(with: data, options:JSONSerialization.ReadingOptions.mutableContainers) as! NSDictionary

                DispatchQueue.main.sync
                    {
                        if (jsonResult.value(forKey: "status") as! String == "OK")
                        {

                            if let routes = jsonResult.value(forKey: "routes") as? NSArray
                            {

                                if let legs = routes.value(forKey: "legs") as? NSArray
                                {

                                    if let duration = legs.value(forKey: "duration_in_traffic") as? NSArray
                                    {

                                        for i in 0..<duration.count
                                        {
                                            let timeDuration = duration[i] as! NSArray

                                            if let time = timeDuration.value(forKey: "value") as? NSArray
                                            {

                                                print("Current Time: \(Int(time[0] as! NSNumber)/60) Min")

                                            }
                                        }

                                    }

                                }

                            }

                        }
                }

It works perfectly fine but it looks like really bad code. But every example i found solves it similar. Has anyone a cleaner solution? Thank you in advance!

Since Swift 1.2, multiple optional bindings and boolean conditions
can be combined to a single if-statement. This helps to avoid the
"Swift if-let pyramid of doom". In your case that would be

if jsonResult.value(forKey: "status") as! String == "OK",
    let routes = jsonResult.value(forKey: "routes") as? NSArray,
    let legs = routes.value(forKey: "legs") as? NSArray,
    let duration = legs.value(forKey: "duration_in_traffic") as? NSArray {

    for i in 0..<duration.count {
        let timeDuration = duration[i] as! NSArray
        if let time = timeDuration.value(forKey: "value") as? NSArray {
            print("Current Time: \(Int(time[0] as! NSNumber)/60) Min")

        }
    }
}

and reduces the maximal nesting level by three.

There are two "forced unwraps" in your code:

let jsonResult = try JSONSerialization.jsonObject(with: data, options:JSONSerialization.ReadingOptions.mutableContainers) as! NSDictionary

and

if jsonResult.value(forKey: "status") as! String == "OK",

which can cause a program termination at runtime. Better use
optional casts. In the second case that would simply be

if jsonResult.value(forKey: "status") as? String == "OK",

because optionals and non-optionals can be compared.

The JSONSerialization.ReadingOptions.mutableContainers option
is not needed because you are not modifying the returned dictionaries
or arrays.

You are using value(forKey:) a lot. This is a method from the
Key-Value Coding protocol, and is often not what you need. In

let routes = jsonResult.value(forKey: "routes")

you are simply retrieving a dictionary value, that is:

let routes = jsonResult.object(forKey: "routes")

In

let legs = routes.value(forKey: "legs")

you are mapping the routes array to a new array legs. This is
Key-Value coding, however it can be confusing, in particular in this
case where legs would be an array of arrays. This makes duration
and timeDuration nested arrays as well, and is the reason why
you have to subscript time[0] finally. If you are interested
in the first route only, then it is easier to extract the first
element of routes and continue to work with that.

Generally it is preferred to use native Swift type instead of
Foundation types, e.g. [Any] (or a more specific type) for an array,
or [String, Any] for a dictionary. Compare
Working with JSON in Swift from the Swift blog for more information.

Instead of

for i in 0..<duration.count {
    let timeDuration = duration[i]
    // ...
}

you can simpler write

for timeDuration in duration {
    // ...
}

Putting it all together, I would write the code like this:

let jsonResult = try JSONSerialization.jsonObject(with: data)

DispatchQueue.main.sync {
    if let jsonDict = jsonResult as? [String: Any],
        jsonDict["status"] as? String == "OK",
        let routes = jsonDict["routes"] as? [[String: Any]],
        let route = routes.first,
        let legs = route["legs"] as? [[String: Any]] {

        for leg in legs {
            if let duration = leg["duration_in_traffic"] as? [String: Any] {
                if let time = duration["value"] as? Int {
                    print("\(time / 60) Min")
                }
            }
        }
    }
}

Note how duration["value"] can be conditionally cast to  an Int
without an intermediate NSNumber.

Finally you can factor the extraction of the duration data into
a separate function:

func durations(from directions: [String: Any]) -> [Int]? {
    guard directions["status"] as? String == "OK",
        let routes = directions["routes"] as? [[String: Any]],
        let route = routes.first,
        let legs = route["legs"] as? [[String: Any]]  else {
            return nil
    }

    return legs
        .flatMap { $0["duration_in_traffic"] as? [String: Any] }
        .flatMap { $0["value"] as? Int }
}

which can then be used as

if let directions = (try JSONSerialization.jsonObject(with: data)) as? [String: Any],
    let durations = durations(from: directions) {
    DispatchQueue.main.sync {
        for time in durations {
            print("\(time / 60) Min")
        }
    }
}

Here flatMap() is used to map each "leg" to its duration,
this is essentially what you did with value(forKey:) in your
original code.


Tweets per second calculator (Talent Buddy) This challenge is from Talent Buddy:

    Tweets per second

  Japan Castle in the Sky airing broke a Twitter record on August 3,
  2013. At one point during the classic anime movie showing, people submitted 143,199 tweets per second. This particular spike was about
  25 times greater than Twitter’s steady state.

  Given an array of integers representing the number of tweets recorded
  every second and an integer value K, your task is to write a function
  that prints to the standard output (stdout) the highest number of
  tweets recorded between each second in the array and the past K
  seconds before it

  Note that your function will receive the following arguments: tps,
  which is an array of integers representing the number of tweets
  recorded every second, and k, which is the integer number mentioned
  above.

  Data constraints: the length of the array above will not exceed
  500,000 numbers

  Efficiency constraints: your function is expected to print the
  requested result and return in less than 2 seconds

  Example Input

  tps: 6, 9, 4, 7, 4, 1

  k: 3 Output

  6 9 9 9 7 7 Please Note: The above program should run in below 2
  seconds

Below is my code in c#

public void tweets_per_second(int[] tps, int k) {
    // Write your code here
    // To print results to the standard output you can use Console.WriteLine()
    // Example: Console.WriteLine("Hello world!");

    for(int i=0; i<tps.Length;i++)
    {
        Console.WriteLine(FindMaxLastK(tps, i, k));

    }
}
// method to check the maximum of last k items in the array
public int FindMaxLastK(int[] tps, int start, int k)
{
    k = k-1;
    int max = 0;
 while(k >= 0 && start >= 0)
 {
     max = tps[start] > max? tps[start]:max;
     k--;
     start--;
 }
    return max;
}

The code has been accepted by the website,  but somehow I feel it can be done better. Any suggestions?

Naming

Your FindMaxLastK method doesn't describe it's intention very well, what is tps and k when viewed at a glance?. What you have is a windowing function finding the max within that window. Name your function after that:

static int MaxInWindow(int[] values, int start, int trailingWindowSize)

The conditional expression in your max function could be clearer if you used Math.Max(int, int)

My implementation would look like:

public static int MaxInWindow(int[] values, int start, int trailingWindowSize)
{
    //TODO Validate arguments
    var lowerBound = Math.Max(start - trailingWindowSize, -1);
    var max = values[start];
    for (int index = start - 1; index > lowerBound; index--)
    {
        max = Math.Max(max, values[index]);
    }
    return max;
}

Performance

Your solution is O(m * n) where m is the number of seconds and n is the window size. I just saw the answer in a related question and the optimal solution is O(m).

The solution uses a LinkedList to keep track of the max in the window. Here is my implementation in C#:

public static IEnumerable<T> MaxInSlidingWindow<T>(T[] values, int windowSize)
{
    var comparer = Comparer<T>.Default;
    var window = new LinkedList<Tuple<T, int>>();// Max at position and position

    var length = values.Length;
    for (var index = 0; index < length; index++)
    {
        var value = values[index];
        while (window.Count != 0 && comparer.Compare(window.Last.Value.Item1, value) < 0)
        {
            window.RemoveLast();
        }

        window.AddLast(Tuple.Create(value, index));

        while (window.First.Value.Item2 <= index - windowSize)
        {
            window.RemoveFirst();
        }

        yield return window.First.Value.Item1;
    }
}


Avoiding casts in abstract types I asked this question on Stack Overflow and in the comments someone had this to say.

  In a proper design, you should almost never have to do a dynamic_cast, even if it's hidden inside some nice getter function.

Now this got me thinking a lot. I have been thinking of various ways in which I could eliminate the dynamic_cast in question, or any cast for that matter from my code. Now I come here rather frustrated as the only thing I can think of to get rid of that cast; in that location is by doing the cast elsewhere.

So I ask, how can I improve upon this design?

#include <iostream>
#include <string>

// Properties for all components
class ComponentProperties {
public:
  ComponentProperties( std::string componentProperty ):
    componentProperty_( componentProperty ) { }

  virtual ~ComponentProperties(  ) {  }
  std::string getComponentProperty(  ) const { return componentProperty_; }

protected:
  std::string componentProperty_;
};

// Properties specific to Textboxes
class TextboxProperties: public ComponentProperties {
public:
  TextboxProperties( std::string componentProperty, std::string textboxProperty ):
    ComponentProperties( componentProperty ),
    textboxProperty_( textboxProperty ) {  }

  std::string getTextboxProperty(  ) { return textboxProperty_; }

private:
  std::string textboxProperty_;
};

class Component {
public:
  Component( ComponentProperties& properties ):
    properties_( properties ) {  }

  virtual ~Component(  ) {  }

protected:
  ComponentProperties& properties_;
};

class Textbox : public Component {
public:
  Textbox( TextboxProperties& properties ):
    Component( properties ) {  }

  // overload << purely for displaying properties.
  friend std::ostream & operator << ( std::ostream& out, const Textbox& textbox );
};

std::ostream & operator << ( std::ostream& out, const Textbox& textbox ) {
  return out << textbox.properties_.getComponentProperty(  ) << ", "
             << dynamic_cast< TextboxProperties& >( textbox.properties_ ).getTextboxProperty(  );
}

int main(  ) {
  TextboxProperties properties( "PropertyOne", "PropertyTwo" );
  Textbox textbox( properties );

  std::cout << textbox << std::endl;
  return 0;
}

The best solution is to make ComponentProperties know how to print itself.

 class ComponentProperties {
     friend std::ostream& operator<<(std::ostream& s, ComponentProperties const& d) {
         return d.print(s);
     }
     virtual std::ostream& print(std::ostream& s) const {
         return s << componentProperty_;
     }
     //  STUFF
 };

 class TextboxProperties: public ComponentProperties {
     virtual std::ostream& print(std::ostream& s) const {
         return ComponentProperties::print(s) << ", " << textboxProperty_;
     }
     // STUFF
  };

  class Component {
      // PS. I tend to put my simple friend class definitions.
      //     inline within the class. They are after all part
      //     of the classes public interface and tightly bound
      //     to the implementation of the class.
      friend std::ostream& operator<<(std::ostream& out, const Component& component) {
          return out << component.properties_;
      }
      // STUFF
  };

Advantage of doing it this way is you can get rid of this getter methods.


Finding all plurals in an array of Strings I'm looking for feedback on this here code. Is readability an issue? Is it good to mess with all these data types in Java (e.g. strings and Chars and using .equals() and != to compare data from different data types), or is there an easier way to do it? And in general, how can I improve this code?

  Write a method named allPlural that accepts an array of strings as a
  parameter and returns true only if every string in the array is a
  plural word, and false otherwise. For this problem a plural word is
  defined as any string that ends with the letter S, case-insensitively.
  The empty string "" is not considered a plural word, but the
  single-letter string "s" or "S" is. Your method should return true if
  passed an empty array (one with 0 elements).

   public static boolean allPlural(String[] words){
        for (String s : words) {
            // blank string is not plural
            if (s.equals("")) {
                return false;
            }
            // if the strings in the array don't have an s on the end, return false
            if (Character.toUpperCase(s.charAt(s.length()-1)) != 'S') {
                return false;
            }
        }
        return true;
    }

The code is well readable as it is. As long as it solves the problem (it does), it is fine.

For a real-world plural detection, I would add to the class:

  public static boolean isPlural(String s)

to detect if a single word is a proper plural, because it is more complicated than 'ends with "S"'.


Mapping square objects to specific cells in a TableLayoutPanel in WindowsForms game I have solved this problem but I am looking for a better way to do it, in fewer lines of code.

The problem was from this previous question on Stack Overflow.

My solution:

   /// <summary>
        /// For a given square number, tells you the corresponding row and column numbers.
        /// Pre:  none.
        /// Post: returns the row and column numbers, via "out" parameters.
        /// </summary>
        /// <param name="squareNumber">The input square number.</param>
        /// <param name="rowNumber">The output row number.</param>
        /// <param name="columnNumber">The output column number.</param>
        private static void MapSquareNumToScreenRowAndColumn(int squareNumber, out int rowNumber, out int columnNumber){

        // ######################## Add more code to this method and replace the next two lines by something more sensible.  ###############################
        rowNumber = 0; // Use 0 to make the compiler happy for now.
        columnNumber = 0; // Use 0 to make the compiler happy for now.

        columnNumber = squareNumber % 6;
        rowNumber = squareNumber/6;

        //switch statement for assigning column values to odd rows
        int remainder = squareNumber%6;
        switch (remainder){
            case 0:
                columnNumber = 5;
                break;
            case 1:
                columnNumber = 4;
                break;
            case 2:
                columnNumber = 3;
                break;
            case 3:
                columnNumber = 2;
                break;
            case 4:
                columnNumber = 1;
                break;
            case 5:
                columnNumber = 0;
                break;

        }

        if (squareNumber >=0 && squareNumber <= 5){
            rowNumber = 6;
            columnNumber = squareNumber%6;

        }

        if (squareNumber >=6 && squareNumber <= 11){
            rowNumber = 5;
        }

        if (squareNumber >=12 && squareNumber <= 17){
            rowNumber = 4;
            columnNumber = squareNumber%6;
        }
        if (squareNumber >=18 && squareNumber <= 23){
            rowNumber = 3;

        }
        if (squareNumber >= 24 && squareNumber <= 29){
            rowNumber = 2;
            columnNumber = squareNumber%6;

        }
        if (squareNumber >= 30 && squareNumber <= 35){
            rowNumber = 1;

        }
        if (squareNumber >= 36 && squareNumber <= 41){
            rowNumber = 0;
            columnNumber = squareNumber % 6;
        }

    }//end MapSquareNumToScreenRowAndColumn

I'm fairly new to programming, so I just wanted a second opinion on it. Is there a way for me to get the same outcome, but with less code?

The problem of your code is that you use separate conditions for each particular row and for each particular column.
But it can be easily aggregated into one - check if the row number is odd. A rest of the desired functionality can be reached using arithmetics.

Please try this code:

private static void MapSquareNumToScreenRowAndColumn(int squareNumber, out int rowNumber, out int columnNumber)
{
    const int Rows = 7;
    const int Cols = 6;

    // Calculate the rowNumber and the columnNumber at once.
    // See Math.DivRem for details: http://msdn.microsoft.com/en-us/library/yda5c8dx(v=vs.110).aspx
    rowNumber = Rows - 1 - Math.DivRem(squareNumber, Cols, out columnNumber);

    // If the rowNumber is odd:
    if ((rowNumber & 1) != 0)  // (rowNumber & 1) is the same as (rowNumber % 2)
        columnNumber = Cols - 1 - columnNumber;  // Invert the columnNumber
}


Python Code Cleanup/Optimization I am trying to clean up and make the following code more efficient -

https://gist.github.com/eWizardII/6757364

from PIL import Image
import numpy as np
from PIL import ImageChops
import subprocess
import math
import datetime
from PIL import ImageFont
from PIL import ImageDraw
from collections import deque

def image_entropy(prev, curr, smasumm, values):
    im1 = Image.open(prev)
    im2 = Image.open(curr)
    img = ImageChops.difference(im1, im2)

    w, h = img.size
    a = np.array(img.convert('RGB')).reshape((w * h, 3))
    h, e = np.histogramdd(a, bins=(16,) * 3, range=((0, 256),) * 3)
    prob = h / np.sum(h) # normalize
    prob = prob[prob > 0] # remove zeros
    comp = -np.sum(prob * np.log2(prob))
    framsd = SD(values, smasumm)
    information = 'ENT: ' + str('{0:.2f}'.format(comp)) + ' SMA: ' + str('{0:.2f}'.format(smasumm)) + ' LB: ' + str(
        '{0:.2f}'.format(smasumm - framsd)) + ' UB: ' + str('{0:.2f}'.format(smasumm + framsd))
    cimg = Image.open(curr)
    draw = ImageDraw.Draw(cimg)
    font = ImageFont.truetype("arial.ttf", 24)
    draw.text((0, 0), information, (0, 255, 0), font=font)
    cimg.save(curr)
    return comp

def SD(values, mean):
    size = len(values)
    sumsd = 0.0
    for n in range(0, size):
        sumsd += math.sqrt((values[n] - mean) ** 2)
    return math.sqrt((1.0 / (size - 1)) * (sumsd / size))

try:

    print "Initial Image ..."

    time = datetime.datetime.now()
    filename = "image_"
    subprocess.call(
        "raspistill -t 1000 -ex night -awb auto -w 720 -h 480 -o %s" % filename + '1.jpg',
        shell=True)
    prev = filename + '1.jpg'
    i = 1
    summ = smasumm = 0.0
    period = n = 10
    values = deque([0.0] * period)
    while True:
        time = datetime.datetime.now()
        filename = "image_" + str(i) + ".jpg"
        subprocess.call(
            "raspistill -t 1000 -ex night -awb auto -w 720 -h 480 -o %s" % filename,
            shell=True)

        i += 1
        curr = filename
        sma = image_entropy(prev, curr, smasumm, values)
        values.append(sma)
        summ += sma - values.popleft()
        smasumm = summ / n
        prev = curr
        print(datetime.datetime.now() - time)
except KeyboardInterrupt:
    print "  Quit"

I believe my biggest bottleneck is in using the subprocess.call in Python but I don't know of any way to improve up such a call - in the long run when I know the program works properly I'll try and transition to Java/C; but this is easier for my immediate application and debugging. Any help or advice would be appreciated thanks in addition to any improvements in the other aspects of the code.

I don't know about "more efficient", but here are some hints and tips that will help you clean up the code and make it more readable:

Style remarks

My style checker asks for at least 2 spaces before an inline comment (lines 20-21). I don't think it's a bad idea.
According to PEP8, you should keep your lines shorter than 80 characters (lines 24, 25 and 49).
This line is not very clear.

information = 'ENT: ' + str('{0:.2f}'.format(comp)) + ' SMA: ' + str('{0:.2f}'.format(smasumm)) + ' LB: ' + str(
 '{0:.2f}'.format(smasumm - framsd)) + ' UB: ' + str('{0:.2f}'.format(smasumm + framsd))

It would be a lot clearer to rewrite it like this:

'ENT: {0:.2f} SMA: {0:.2f} LB: {0:.2f} UB: {0:.2f}'.format(
    comp, smasumm, smasumm - framsd, smasumm + framsd)

Avoid one letter variable names (and other cryptic names like smasumm,  because they're not descriptive and it's not immediately obvious what they represent.
The lack of comments makes it difficult to follow what you're trying to do. You should use docstrings to write a small paragraph that provides an explanation of what you're trying to do.
Again according to PEP8:

  Imports should be grouped in the following order:

  standard library imports
  related third party imports
  local application/library specific imports

So you want to replace your imports with this:

from collections import deque
import datetime
import math
import subprocess

import numpy as np
from PIL import Image
from PIL import ImageChops
from PIL import ImageDraw
from PIL import ImageFont

Why do you alias numpy as np? It's not very readable. I'd rather read numpy in full letters each time.


Simple string compression reloaded Inspired by this question I thought I provide my implementation. I tried to go with the spirit of the *nix tool chain - read from stdin and write to stdout. This has the added benefit of making buffering very easy (current and previous characters and the count).

All kinds of reviews welcome (best practices, error handling, weird edge cases, potential bugs or other pitfalls).

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

void write_char(int c)
{
    if (EOF == putchar(c))
    {
        if (ferror(stdout))
        {
            perror("error writing char to stdout");
            exit(EXIT_FAILURE);
        }
    }
}

void write_count(uint64_t count)
{
    if (printf("%ull", count) < 0)
    {
        perror("error writing character count to stdout");
        exit(EXIT_FAILURE);
    }
}

int main(int argc, char** argv)
{
    int current_char = 0;
    int previous_char = 0;
    uint64_t current_char_count = 0;

    while (EOF != (current_char = getchar())
    {
        if (current_char_count == 0 || current_char_count == UINT64_MAX || previous_char != current_char)
        {
            if (current_char_count > 0)
            {
                write_count(current_char_count);
            }
            write_char(current_char);
            current_char_count = 1;
            previous_char = current_char;
        }
        else
        {
            current_char_count += 1;
        }
    }
}

Compressor number or real
When you are write_counting, you are writing the ASCII number characters to the new file. However, when you go to decompress this file, how are you going to differentiate between the actual content in the file and the numbers that mark the occurrences of a character?
A possible solution for this might be to just write the number itself to the file (no ASCII). That way, when you encounter a number that is ASCII, you can be almost sure that the number is part of the content (that is, unless there was a letter that occurred so many times in a row that the counter rose into the '0'-'9' range).

Two ones or twelve?
This is kind of a continuation from the top one. Let's say your compressor went to go compress this file:

12

Now, I am ready to decompress it. Since your compressor writes a number to show occurrences of a character, the output would be this:

1121

How do I know if all of those numbers are part of the content?
The only fix I can think of, unfortunately, would be to follow the above tip and write 0x01 instead of an ASCII number.

Misc

while (EOF != (current_char = getchar())

You are missing a brace here.

if (printf("%ull", count) < 0)

When compiling your code, I get this on this line:

warning: format ‘%u’ expects argument of type ‘unsigned int’, but argument 2 has type ‘uint64_t’ [-Wformat=]

This also showed a problem that two ls are written after the number that shows how many character occurrences there were.


My first implementation of a client/server application Well, it's 2 applications really. Each one does its own stuff on the front-end. It's the networking that I'm most concerned about.

The Server starts listening for connections on the specified port as soon as it starts up. When clients connect, it accepts, serves and boots them in turn (or rather that's the idea but I think there's a flaw in what I've got here).

The server is multithreaded to handle requests on new threads so as to avoid locking its UI while it works.

public void StartListening()
{
    TcpListener Svr = null;

    Int32 Port = 13000;
    IPAddress LocalAddress = IPAddress.Parse("127.0.0.1");

    Svr = new TcpListener(LocalAddress, Port);

    Svr.Start();

    Byte[] Buffer = new Byte[1024];

    Task.Run(() =>
    {
        while (true)
        {
            TcpClient Client = Svr.AcceptTcpClient();
            NetworkStream Stream = Client.GetStream();

            byte[] Response = HandleClientData(Stream, Buffer, Client);

            Stream.Write(Response, 0, Response.Length);

            Client.Close();
        }
    });
}

The function that reads and works with the data HandleClientData(NetworkStream, byte[], TcpClient) looks like this:

private byte[] HandleClientData(NetworkStream stream, Byte[] buffer, TcpClient client)
{
    int i;

    byte[] value = null;

    while ((i = stream.Read(buffer, 0, buffer.Length)) != 0)
    {
        string ClientData = Encoding.ASCII.GetString(buffer, 0, i);

        // do stuff with the client data and send a response
        value = ...;
    }
return value;
}

The Client is much the same, except its not listening for connections. It's only job is to send requests to the server and display the result that the server sends back:

public class _TcpClient
{
    public const string Server = "127.0.0.1";
    public const int Port = 13000;

    public string SendData(string data)
    {
        try
        {
            string Response = String.Empty;
            using (TcpClient Client = new TcpClient())
            {
                Client.Connect(Server, Port);

                using (NetworkStream Stream = new NetworkStream(Client.Client))
                {
                    Stream.Write(Encoding.ASCII.GetBytes(data), 0, 256);

                    int ByteRead = 0;
                    byte[] Buffer = new Byte[256];

                    while (ByteRead > 0)
                    {
                        ByteRead = Stream.Read(Buffer, 0, 256);
                        Response += Encoding.ASCII.GetString(Buffer, 0, ByteRead);
                    }
                }
            }

            return Response;
        }
        catch (Exception ex)
        {
            return String.Format("ERR,{0}", ex.Message);
        }
    }
}

To keep things simple, all comms between client and server send and receive delimited strings which are read by each component and handled in a particular way for each.

Is this a fairly decent implementation or a client/server program? How can I improve this or make it more functional?

Naming

Usually, local variables start with a small letter.

StartListening

If the method is called multiple times, it will throw an exception that the endpoint is already in use. Not very robust.
Task.Run(action) uses a thread from the thread pool. Those threads are usually pooled for short running actions. TaskFactory.StartNew(action, TaskCreationOptions.LongRunning) creates a new thread to run the action in. That is more appropriated for long running tasks.
You should use try / catch or using to ensure that the client will be disposed, even if an error occurs.
You should provide a way to shut down the server - closing the application is IMHO not the right way.

HandleClientData

If the incoming data size is bigger than 1024, only the last chunk is received. You would probably add the chunk to the string: clientData += Encoding.ASCII.GetString(buffer, 0, i);


Stack implementation in C++ including min(), max() functions in O(1) This implementation seems to work. I would love to get any suggestions / comments on how this code could be improved. Are there any potential memory management issues? Any additional methods that would be useful (also, from educational point of view) to implement? Thank you!

Stack.hpp

#ifndef STACK_H
#define STACK_H

#include <iostream>
#include <memory>
#include <vector>
#include <stdexcept>

class Stack {

private:
    struct Node {
        int val;
        // To get min and max value of stack in O(1):
        int maxVal = std::numeric_limits<int>::min();
        int minVal = std::numeric_limits<int>::max();
    };

    std::vector<Node> currStack;

public:

    int size() const { return currStack.size(); }
    bool empty() const { return currStack.empty(); }
    void pop() { currStack.pop_back(); }

    void push(int val);
    int top() const; // O(1)
    int min() const; // O(1)
    int max() const; // O(1)

};

#endif /* STACK_H */

Stack.cpp

#include "Stack.hpp"

void Stack::push(int val) {

    Node tempNode;
    tempNode.val = val;
    if (this->empty()) {
        tempNode.minVal = val;
        tempNode.maxVal = val;
    } else {
        tempNode.minVal = std::min(currStack.back().minVal, val);
        tempNode.maxVal = std::max(currStack.back().maxVal, val);
    }

    currStack.push_back(tempNode);
}

int Stack::top() const {
    if (currStack.empty()) {
        throw std::out_of_range ("Stack is empty");
    }
    return currStack.back().val;
}

int Stack::min() const {
    if (currStack.empty()) {
        throw std::out_of_range ("Stack is empty");
    }
    return currStack.back().minVal;
}

int Stack::max() const {
    if (currStack.empty()) {
        throw std::out_of_range ("Stack is empty");
    }
    return currStack.back().maxVal;
}

main.cpp

#include "Stack.hpp"

int main() {

    // Stack init
    Stack myStack;

    std::cout << "Empty? " << myStack.empty() << "\n";
    std::cout << "Size: " << myStack.size() << "\n";

    try {
        std::cout << "Top: " << myStack.top() << "\n";
        std::cout << "Min: " << myStack.min() << "\n";
        std::cout << "Max: " << myStack.max() << "\n";
    }
    catch (const std::out_of_range& e) {
        std::cout << "Out of range error. " << e.what() << "\n";
    }

    std::cout << "Push some integers: \n";
    myStack.push(5);
    myStack.push(3);
    myStack.push(7);

    std::cout << "Empty? " << myStack.empty() << "\n";
    std::cout << "Size: " << myStack.size() << "\n";

    try {
        std::cout << "Top: " << myStack.top() << "\n";
        std::cout << "Min: " << myStack.min() << "\n";
        std::cout << "Max: " << myStack.max() << "\n";
    }
    catch (const std::out_of_range& e) {
        std::cout << "Out of range error. " << e.what() << "\n";
    }

    std::cout << "Pop some integers: \n";
    myStack.pop();

    std::cout << "Empty? " << myStack.empty() << "\n";
    std::cout << "Size: " << myStack.size() << "\n";

    try {
        std::cout << "Top: " << myStack.top() << "\n";
        std::cout << "Min: " << myStack.min() << "\n";
        std::cout << "Max: " << myStack.max() << "\n";
    }
    catch (const std::out_of_range& e) {
        std::cout << "Out of range error. " << e.what() << "\n";
    }
}

I'm going to answer your direct questions first.

  Are there any potential memory management issues?

No. You don't do any manual memory management (using new and delete) so there are no issues.

  Any additional methods that would be useful (also, from educational
  point of view) to implement?

Here is the reference page for std::stack. This is the standard library documentation. You may not be able to do them all right now but those are all possible functions and members you could implement for a stack. (I recommend only trying a little bit at a time.

I'd like to point out that your code is very clean and easy to read. I also appreciate good use of const correctness.

Use unique include guards

#ifndef STACK_H
#define STACK_H

This is a very basic name. It would be easy for others to use a very similar name. Make your includes unique. Add a namespace name, a project name, a library name and possibly a GUID or all of the above to eliminate the risk of clashes.

Use the right headers

#include <iostream>
#include <memory>
#include <vector>
#include <stdexcept>

You don't use <memory> or <iostream> in your class. You don't use <stdexcept> until the cpp file. declare the header there.

You are missing <limits> for :std::numeric_limits<>

List public members first

Anyone using your stack is only going to be able to use the public member functions. It is common practice therefor to list these first.

Avoid the use of this->

It is almost always wrong to have this-> in your code in C++. For example this:

if (this->empty())

can simply be written:

if (empty())

The exception to this rule is with variable shadowing (which you don't have) which is also typically solved with a name change.

Always ask: What does this feature serve?

So you implemented a min() and max(). You did it with an O(1) lookup by having every single element keep track of the current min and max value of the entire stack. This is a problem on two levels:

Your nodes should not contain information on other nodes. That breaks encapsulation. This is the job of the Container.
And its also useless information. There is one accessible node. What value do you gain by knowing what the min and max values beneath that one are? how can you even act on that even if you knew?

If you have your heart content on keeping the min() max() then at least store the value in the container. In addition to being good practice it will also take significantly less memory.

Lastly When you are ready you should look into Template Metaprogramming. A Container / Data Structure like this needs to be usable with any type, including user defined types, in order to be truly useful/robust.

Edit: to answer point brought up in comments

The problem as I see it is the fact that you need to store state, which a stack is great for. If I needed to maintain current, min, and max values like you are, I would put them all into a struct like so:

struct State
{
     int current_value;
     int min_value;
     int max_value;
}

Then I would use a stack that supports user-defined types. std::stack<State> state_machine.

Furthermore by maintaining min() and max(), in the node or in the container, if you do decide to extend your container to support user-defined types you will force your users to overload the comparison operators for their type even if there is no logical way to do so and it isn't otherwise needed.


Struct to web style string I've had to make several functions to turn some structures into strings. I am a still green when it comes C so I am unsure if I am doing this a very awkward way. The system I am coding for does not have snprintf, I know that would be far more elegant, however I cannot use it.
Any advice?

int device_to_string(char* const asString, pDevice dev, size_t maxLength)
{
  char* ipAsString;
  size_t actualLength;
  struct in_addr addr;

  if (dev == NULL)
  {
    return NULL_ERROR;
  }

  addr.s_addr = dev->ip;
  ipAsString = inet_ntoa(addr);

  actualLength = strlen("name=")  + strlen(dev->name) +
                 strlen("&ip=")   + strlen(ipAsString) +
                 strlen("&mac=")  + strlen(dev->mac)  +
                 strlen("&type=") + strlen(dev->type) + 1;

  if (actualLength > maxLength)
  {
    return SIZE_ERROR;
  }

  strncat(asString, "name=",    strlen("name="));
  strncat(asString, dev->name,  strlen(dev->name));
  strncat(asString, "&ip=",     strlen("&ip="));
  strncat(asString, ipAsString, strlen(ipAsString));
  strncat(asString, "&mac=",    strlen("&mac="));
  strncat(asString, dev->mac,   strlen(dev->mac));
  strncat(asString, "&type=",   strlen("&type="));
  strncat(asString, dev->type,  strlen(dev->type));

  asString[actualLength] = '\0';

  return NO_ERROR;
}

Yeah, without snprintf and sprintf it gets a bit tedious, but I think this code is actually quite clear. You use your horizontal and vertical whitespace very well, and it's clear what you're doing with each block of code. You have also controlled for any possible issues that might come up (null pointer, insufficient buffer length, etc). Maybe there's a more concise way to do it, but in terms of clarity and maintainability I think this code will suffice.


Process strings of operations This was the task (I don't know the exact wordings anymore):

  You get a string with operation as input:

  if it is a number push it to stack
  if it is "DUP", then duplicate the one one top and push it to the
  stack
  if it is "POP" then remove the one on top
  if it is "+" add the two numbers on top of the stack and replace
  both with the result of the addition
  if it is "-" then minus the two numbers on top (the second minus
  the first) of the stack and replace both with the result of the
  subtraction
  if an error occurs, e.g. not enough numbers to add/subtract/pop,
  then return -1

Examples:

Input: "3 DUP 5 - -"
Output: -1

Input: "13 DUP 4 POP 5 DUP + DUP + -"
Output: 7

Input: "5 6 + -"
Output: -1

My approach is quite simple. I differentiate between numbers and operations. If it is a number I simply push it to the stack. Otherwise Ill check what kind of operation it is. If a valid operation is recognized, then the mapped function will be executed.

If an error occurs, e.g. no more numbers in the stack to be popped, invalid operation, etc. it will return -1.

My solution

function solution(S) {
    if (!S.length) {
        return -1;
    }
    const stack = new Array();
    const push = n => stack.push(Number(n));
    const pop = () => {
        if (!stack.length) {
            throw new Error('empty');
        }
        return stack.pop();
    }
    const duplicate = () => {
        try {
            const dup = pop();
            push(dup);
            push(dup);
        } catch(e) {
            throw new Error(e);
        }
    };
    const add = () => {
        try {
            const sum1 = pop();
            const sum2 = pop();
            push(sum1 + sum2);
        } catch(e) {
            throw new Error(e);
        }
    };
    const minus = () => {
        try {
            const min1 = pop();
            const min2 = pop();
            push(min1 - min2);
        } catch(e) {
            throw new Error(e);
        }
    };
    const ops = {
        "DUP": duplicate,
        "POP": pop,
        "+": add,
        "-": minus,
    };

    const input = S.split(' ');
    input.forEach(x => {
        if (!isNaN(x)) {
            push(x);
        } else {
            try {
                const sanitzedX = x.toUpperCase();
                if (!ops[sanitzedX]) {
                    return -1;
                }
                ops[sanitzedX]();
            } catch(e) {
                return -1;
            }
        }
    });
    try {
        return pop();
    } catch(e) {
        return -1;
    }
}

console.log(solution("3 DUP 5 - -"));
console.log(solution("13 DUP 4 POP 5 DUP + DUP + -"));
console.log(solution("5 6 + -"));

const stack = new Array()

Use the literal notation ([]) instead. Here's an answer on Stack Overflow that elaborates why you should do that instead of new Array().

const duplicate = () => {
    try {
        const dup = pop();
        push(dup);
        push(dup);
    } catch(e) {
        throw new Error(e);
    }
};

The rule of thumb when it comes to using exceptions is to never use them where you can use an if. That is, don't use exceptions as a flow control mechanism. You should only use it if you want to bail out and do no further.

Also, e is already an instance of Error. You only need to do throw e, not throw new Error(e).

Now if you look closely, all the operations are distinct to each other. They also only operate on the stack only. You can easily implement them as pure functions, taking in the current stack, and returning a new representation of the stack.

For example, duplicate can be implemented as a function that takes an array, and returns a new array, with the last item repeated once more:

const duplicate = stack =>
  stack.length < 1 ? -1
  : [...stack, stack[stack.length - 1]]

add can be implemented as a function that takes an array, and return an new array which replaces the last two items with their sum:

const add = stack =>
  stack.length < 2 ? -1
  : [...stack.slice(0, -2), stack[stack.length - 1] + stack[stack.length - 2]]

Doing it this way makes it easier to implement each operation without having them becoming dependent on some common state object, or each other.

Here's how I would implement it:

// Implement each operation as distinct, pure functions.
const push = (stack, v) => [...stack, v]

const pop = stack =>
  stack.length < 1 ? -1
  : stack.slice(0, -1)

const duplicate = stack =>
  stack.length < 1 ? -1
  : [...stack, stack[stack.length - 1]]

const add = stack =>
  stack.length < 2 ? -1
  : [...stack.slice(0, -2), stack[stack.length - 1] + stack[stack.length - 2]]

const subtract = stack =>
  stack.length < 2 ? -1
  : [...stack.slice(0, -2), stack[stack.length - 1] - stack[stack.length - 2]]

// Map operation tokens to their implementation.
// Should we add more operations, we simply add the implementation and reference
// its token in this map
const operations = {
  'DUP': duplicate,
  'POP': pop,
  '+': add,
  '-': subtract
}

const solution = s => {

  // For each token, parse and process.
  // You can implement this piece using iteration functions, loops, or even recursion.
  // I'm just more accustomed to reduce because it can carry a value across iterations
  // which, in this case, holds the stack for me.
  const result = s.split(' ').reduce((stack, token) => {

    // If any operation returned -1, we return -1 all the way.
    return stack === -1 ? -1
      // If token is a number, push it to the stack.
      : !isNaN(parseFloat(token)) && isFinite(token) ? push(stack, Number(token))
      // If token is an operation, apply operation.
      : operations[token.toUpperCase()] ? operations[token.toUpperCase()](stack)
      // Unknown token.
      : -1

  // Our stack as initial reduce value, an empty array.
  }, [])

  // If the result is -1, then so be it. Otherwise, return the top of the stack.
  return result === -1 ? -1 : result[result.length - 1]
}

console.log(solution("3 DUP 5 - -"));
console.log(solution("13 DUP 4 POP 5 DUP + DUP + -"));
console.log(solution("5 6 + -"));


Selecting a quantity from a list I want to implement a function take_upto_n(A, n)
such that for e.g.
take_upto_n([1,2,3,4], 8) returns [1,2,3,2]
take_upto_n([1,2,4,3], 8) returns [1,2,4,1]
take_upto_n([1,2,3,4], 100) returns [1,2,3,4]
take_upto_n([1,2,3,4], 3) returns [1,2,0,0]

Assume A contains non-negative values only. n is also non-negative
This is straightforward with a loop
def take_upto_n(A, n):
    out = []
    for x in A:
        out += [min(n, x)]
        n -= min(n, x)
    return out

We can also do this with numpy, but this I.M.O. is fairly unreadable:
def take_upto_n(A, n):
    A = np.array(A)
    return np.clip(n + A - np.cumsum(A), 0, A)

Is there some standard function that does this - couldn't find it myself (or didn't know how to search)?
If there isn't any such function, any advice on how to make it not look like "what is this?" (the n + A - np.cumsum(A) is the real killer)?

Your regular Python implementation generally looks reasonable, and unless numpy
offers a performance boost that you really need, I would not recommend it for
this use case: the brevity/clarity tradeoff seems bad.
My biggest suggestion is to consider clearer names. A function with signature
of take_upto_n(A, n) makes me think the function takes an iterable and
returns a sequence of n values. Something like take_upto_sum(A, total)
seems more communicative to me. And even "take" isn't quite right: take
implies a filtering process (we'll take some and leave some). But we
are limiting or capping. Perhaps you can think of something even better.
This example presents a classic Python dilemma: you want to do a simple map
over a list to generate a new list, so of course you would like to knock it out
in a comprehension or something equally cool. But while iterating,
you also need to modify some other variable to keep track of state, and Python
mostly lacks the assignments-as-expression idiom that allows such trickery in
other languages (for example, old-school Perl map/grep one-liners and their
ilk). If you are using a sufficiently modern Python, you can use the walrus
operator to compress the code a bit, but it doesn't get you all the way there.
Honestly, I wouldn't even bother with the walrus (but I would assign min() to
a convenience variable to avoid repetition and thus help readability). In any
case, the walrus approach:
def take_upto_sum(A, total):
    out = []
    for x in A:
        out.append(y := min(total, x))
        total -= y
    return out

I suppose it is possible to express this as a single list comprehension if we
take advantage of the built-in itertools and operator modules. Like the
numpy solution, this approach reminds me of the Spinal Tap quote: It's such a
fine line between stupid, and uh ... clever.
from itertools import accumulate
from operator import sub

def take_upto_sum(A, total):
    return [
        max(min(x, acc), 0)
        for x, acc in zip(A, accumulate(A, sub, initial=total))
    ]


Oracle SQL multiple count query There is an Employee table with data like:

ID    FIRST_NAME     END_DATE                      CITY
23    Manoj       24-JUL-16 08.45.02.000000 AM    Bangalore
22    Abhishek    24-JUL-16 08.45.01.000000 AM    Bangalore
24    Nilu        24-JUL-16 08.46.01.000000 AM    Bangalore
25    Niroj       24-JUL-16 12.08.43.000000 PM    Bangalore
26    Tulu        24-JUL-16 10.47.01.000000 AM    Bangalore
29    Prashant    24-JUL-16 10.50.01.000000 AM    Bangalore
27    Tulu        24-JUL-16 01.32.01.000000 AM    Chennai
28    Panjvir     24-JUL-16 09.50.01.000000 AM    Bangalore

I need results like group by city and number the records for last minute, last sec, last hour and today with comparing to end date.

I am able to get the results with this query:

select e1.city,
       (select count(*) from Employee where end_date > (sysdate - interval '1' minute) and city = e1.city) as las_min,
       (SELECT count(*) FROM Employee WHERE end_date > (sysdate - interval '1' hour) and city = e1.city) as last_hours,
       (select count(*) from Employee where TRUNC(end_date) <= sysdate and city = e1.city) as today,
       (select count(*) from Employee where end_date between add_months(trunc(sysdate,'mm'),-1) and last_day(add_months(trunc(sysdate,'mm'),-1)) and city = e1.city) as last_months
 from Employee e1 group by e1.city;

Is there any better way to get the same results?

Consider using conditional aggregations to avoid the multiple subqueries:

SELECT e1.city,
    SUM(CASE end_date > (sysdate - interval '1' minute) THEN 1 ELSE NULL END) as las_min,
    SUM(CASE end_date > (sysdate - interval '1' hour) THEN 1 ELSE NULL END) as last_hours,
    SUM(CASE TRUNC(end_date) <= sysdate THEN 1 ELSE NULL END) as today,
    SUM(CASE end_date BETWEEN add_months(trunc(sysdate,'mm'),-1)
        AND last_day(add_months(trunc(sysdate,'mm'),-1)) THEN 1 ELSE NULL END) as last_months
FROM Employee e1
GROUP BY e1.city;

And even shorter, sum the logical expressions:

SELECT e1.city,
       SUM(end_date > (sysdate - interval '1' minute)) as las_min,
       SUM(end_date > (sysdate - interval '1' hour)) as last_hours,
       SUM(TRUNC(end_date) <= sysdate) as today,
       SUM(end_date BETWEEN add_months(trunc(sysdate,'mm'),-1)
           AND last_day(add_months(trunc(sysdate,'mm'),-1))) as last_months
FROM Employee e1
GROUP BY e1.city;


Drawing dragon curves using Turtle graphics This is exercise 3.2.23. from the book Computer Science An Interdisciplinary Approach by Sedgewick & Wayne:
Write a recursive Turtle client that draws dragon fractal.
The following is the data-type implementation for Turtle graphics from the book which I beautified:
public class Turtle {
    private double x;
    private double y;
    private double angle;

    public Turtle(double x, double y, double angle) {
        this.x = x;
        this.y = y;
        this.angle = angle;
    }
    public void turnLeft(double delta) {
        angle += delta;
    }
    public void goForward(double step) {
        double oldX = x, oldY = y;
        x += step * Math.cos(Math.toRadians(angle));
        y += step * Math.sin(Math.toRadians(angle));
        StdDraw.line(oldX, oldY, x, y);
    }
}

StdDraw is a simple API written by the authors of the book.
Here is my program:
public class Dragon {
    public static void drawDragonCurve(int n, double step, Turtle turtle) {
        if (n == 0) {
            turtle.goForward(step);
            return;
        }
        drawDragonCurve(n - 1, step, turtle);
        turtle.turnLeft(90);
        drawNodragCurve(n - 1, step, turtle);

    }
    public static void drawNodragCurve(int n, double step, Turtle turtle) {
        if (n == 0) {
            turtle.goForward(step);
            return;
        }
        drawDragonCurve(n - 1, step, turtle);
        turtle.turnLeft(-90);
        drawNodragCurve(n - 1, step, turtle);
    }
    public static void main(String[] args) {
        int n = Integer.parseInt(args[0]);
        double step = Double.parseDouble(args[1]);
        Turtle turtle = new Turtle(0.67, 0.5, 0);
        drawDragonCurve(n, step, turtle);
    }
}

I checked my program and it works. Here is one instance of it:
Input: 12 0.007
Output:

Is there any way that I can improve my program?
Thanks for your attention.

Whitespace
There should be a blank line between two method definitions.
Also, some blank lines inside of the methods would give the code room to breathe, and allow you to visually separate individual "steps" from each other, for example in your main method:
int n = Integer.parseInt(args[0]);
double step = Double.parseDouble(args[1]);
Turtle turtle = new Turtle(0.67, 0.5, 0);

drawDragonCurve(n, step, turtle);

Magic values
There are several magic values in your code. While it is probably clear what 90 means in a turtle graphic environment, the values 0.67 and 0.5 mean nothing to me. What are they? And more importantly: Why are they the values that they are? Why not 0.66 or 0.68?
Variables allow you to give explanatory, intention-revealing names to your values.
final var initialXCoordinate = 2.0 / 3.0;
final var initialYCoordinate = 0.5;
final var initialAngle = 0;
turtle = new Turtle(initialXCoordinate, initialYCoordinate, initialAngle);

Type inference
I am a big fan of type inference, especially in cases where the type is obvious:
double step = Double.parseDouble(args[1]);

How often do I have to told that this is a double? I get it!
var step = Double.parseDouble(args[1]);

Same here:
Turtle turtle = new Turtle(0.67, 0.5, 0);

This almost sounds like someone stuttering. I don't need the turtle variable to be explicitly annotated with the Turtle type to understand that a variable called turtle being initialized with a Turtle is probably a turtle:
var turtle = new Turtle(0.67, 0.5, 0);

final
I am also a big fan of making everything that can be made final explicitly final. And even for things that can't be made final as written, I'd investigate whether it can be rewritten so it can be made final.
Note, by "everything" I mean primarily variables and fields. However, unless a class is explicitly designed to be extended, it should also be marked final. And of course, immutable classes need to be final anyway.
private
None of your methods are used by any outside client (except for main), so they should all be private.
Naming
n is a terrible name. I have no idea what it means. I promise you, your keyboard is not going to wear out from making your variable names a little more descriptive. I believe it is referring to the order of the fractal.
Also, I have no idea what a Nodrag is.
Scope
It seems that step is always the same. There is no need to pass it as an argument if it is always the same. The purpose of a parameter is to allow a method to differ in its behavior. There is no need for it if it is never different.
The same applies to Turtle.
Code Duplication
This is the big one. There is a lot of code duplication. In fact, drawDragonCurve and drawNodragCurve are 100% identical except for one single character.
The only thing that is changing is the sign of the turn angle, or in other words, the direction of the turn. We can turn the sign into a parameter of the method, and pass it along accordingly.
I will keep the original drawDragonCurve method in place as an overload, which calls the new overload with the correct starting parameter, because a client should not need to know whether 1 or -1 is the correct starting value.
public class Dragon {
    private static double step;
    private static Turtle turtle;

    private static void drawDragonCurve(final int order) {
        drawDragonCurve(order - 1, 1);
    }

    private static void drawDragonCurve(final int order, final int sign) {
        if (order == 0) {
            turtle.goForward(step);
            return;
        }

        drawDragonCurve(order - 1, 1);
        turtle.turnLeft(sign * -90);
        drawDragonCurve(order - 1, -1);
    }

    public static void main(final String[] args) {
        final var order = Integer.parseInt(args[0]);
        step = Double.parseDouble(args[1]);

        final var initialXCoordinate = 2.0 / 3.0;
        final var initialYCoordinate = 0.5;
        final var initialAngle = 0;
        turtle = new Turtle(initialXCoordinate, initialYCoordinate, initialAngle);

        drawDragonCurve(order);
    }
}

Note that I might have made a mistake with the flipping of the sign. However, there is no way for me to test your code, because Dragon is intimately linked to Turtle and Turtle is intimately linked to StdDraw, which I don't have access to.
This brings me to my last point:
Testability, Modularity, Overall Design

All your methods are static, in other words, they aren't really methods at all, they are glorified procedures. There are no objects!
All your methods return void, in other words, they don't return anything, they purely perform side-effects.
All your classes are intricately linked to each other, there is no way to separate them, to use them independently, to test them independently.

It is hard to test code if I can't instantiate an object that I can test. It is hard to test code if everything is just a side-effect, and I can't simply compare return values. It is hard to test code if I always need the entire thing and can't test pieces independently or swap out pieces for test versions.
At the moment, the only way to test your code, is to run the entire thing, take a screenshot and compare it with a pre-recorded one. This adds a huge amount of complexity to the tests, and is very slow. Ideally, you want to be able to run your tests every couple of seconds.
For example, Dragon should be a true object, not a static class. It should have the dependency on Turtle injected, not hard-coded. The Turtle dependency should be abstracted behind an interface, so that, for testing purposes, I can replace the turtle with a version that records the commands and compares them to a pre-recorded sequence instead of drawing on the screen.
Likewise, StdDraw should be a true object as well, hidden behind an interface, and I should be able to instantiate a version that uses ASCII art instead of Swing, or that simply creates a bit matrix that I can compare with a pre-recorded one for testing.
At least, Turtle is a class that can actually be instantiated, and has behavior.
This is just a sketch:
App.java
class App {
    public static void main(final String[] args) {
        final var order = Integer.parseInt(args[0]);
        final var step = Double.parseDouble(args[1]);

        final var initialXCoordinate = 2.0 / 3.0;
        final var initialYCoordinate = 0.5;
        final var initialAngle = 0;
        final var turtle = new StdDrawTurtle(initialXCoordinate, initialYCoordinate, initialAngle);

        final var dragon = new Dragon(order, step, turtle);

        dragon.drawDragonCurve();
    }
}

Dragon.java
public record Dragon(final int order, final double step, final Turtle turtle) {
    public void drawDragonCurve() {
        drawDragonCurve(order - 1, 1);
    }

    private void drawDragonCurve(final int order, final int sign) {
        if (order == 0) {
            turtle.goForward(step);
            return;
        }

        drawDragonCurve(order - 1, 1);
        turtle.turnLeft(sign * -90);
        drawDragonCurve(order - 1, -1);
    }

Turtle.java
public interface Turtle {
    public void turnLeft(final double delta);
    public void goForward(final double step);
}

StdDrawTurtle.java
public class StdDrawTurtle implements Turtle {
    private double x;
    private double y;
    private double angle;

    public StdDrawTurtle(final double x, final double y, final double angle) {
        this.x = x;
        this.y = y;
        this.angle = angle;
    }

    public void turnLeft(final double delta) {
        angle += delta;
    }

    public void goForward(final double step) {
        final double oldX = x, oldY = y;

        x += step * Math.cos(Math.toRadians(angle));
        y += step * Math.sin(Math.toRadians(angle));

        StdDraw.line(oldX, oldY, x, y);
    }
}

FakeTurtle.java
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

public class FakeTurtle implements Turtle {
    record TurtleState(double x1, double y1, double x2, double y2) {}

    private final List<TurtleState> recording;
    private double x;
    private double y;
    private double angle;

    public FakeTurtle(final double x, final double y, final double angle) {
        recording = new ArrayList<>();
        this.x = x;
        this.y = y;
        this.angle = angle;
    }

    public List<TurtleState> getRecording() {
        return Collections.unmodifiableList(recording);
    }

    public void turnLeft(final double delta) {
        angle += delta;
    }

    public void goForward(final double step) {
        final double oldX = x, oldY = y;

        x += step * Math.cos(Math.toRadians(angle));
        y += step * Math.sin(Math.toRadians(angle));

        recording.add(new TurtleState(oldX, oldY, x, y));
    }
}
```


Random item generation system to be used in a roguelike New to programming with the goal being to create my own roguelike. I am mostly interested right now in creating my item generation system. I want the code to create a random item that can then be used, stored in an inventory etc. Want to know if the below is a smart way to begin with. Any suggestions to make the below better would be much appreciated.

The item description within function is just there as placeholder so I can see if the code was working. Will this object be permanent ? IE everytime I call create_item it will create a permanent object - Sorry for terminology and if incorrect legit second week of playing with Python :)

import random

i_quality = ["Good", "Bad"]
i_quality = random.choice(i_quality)
i_base = ["Sword", "Gun"]
i_base = random.choice(i_base)
i_element = ["Fire", "Water"]
i_element = random.choice(i_element)

class Item(object):
    def __init__ (self, quality, base, element):
        self.quality = quality
        self.base = base
        self.element = element

def create_Item():
    new_item = Item(i_quality, i_base, i_element)
    i_description = (new_item.quality + " " + new_item.base + " of " + new_item.element)
    print (i_description)

create_Item()

This isn't bad code for someone learning the language, but there are a few things I'd change:

At the top you have:

i_quality = ["Good", "Bad"]
i_quality = random.choice(i_quality)

And other such lines. The main problem here is you're overwriting i_quality. What if you wanted to generate a second quality later? You overwrote your master list of qualities. Have them as separate variables:

all_qualities = ["Good", "Bad"]
all_bases = ["Sword", "Gun"]
all_elements = ["Fire", "Water"]

i_quality = random.choice(all_qualities)
i_base = random.choice(all_bases)
i_element = random.choice(all_elements)

Be careful how you name things and what the responsibilities of functions are. create_Item does create an Item, but it also prints it out then discards it. Such a function isn't very useful. The caller could have just called the Item constructor themselves and then printed the object if they wanted to. Your function does some formatting, but that would be better off in the __str__ method of Item.

What would make for a useful factory function though would be a create_random_item function that returns a random Item. Taking all that together, I'd change this code to something closer to:

import random

all_qualities = ["Good", "Bad"]
all_bases = ["Sword", "Gun"]
all_elements = ["Fire", "Water"]

class Item(object):
    def __init__ (self, quality, base, element):
        self.quality = quality
        self.base = base
        self.element = element

    # What Python will call when you ask it to
    #  display an Item using str
    def __str__(self):
        return self.quality + " " + self.base + " of " + self.element

def create_random_item():
    quality = random.choice(all_qualities)
    base = random.choice(all_bases)
    element = random.choice(all_elements)

    return Item(quality, base, element)

for _ in range(10): # We'll create 10 random weapons and print them out
    item = create_random_item()
    print(str(item)) # str calls item's __str__

Good Gun of Fire
Good Sword of Fire
Bad Sword of Water
Good Sword of Fire
Good Sword of Water
Good Sword of Fire
Bad Gun of Water
Bad Sword of Water
Bad Gun of Water
Bad Sword of Water

This could be further improved though. Any time you have a closed set of members of a set (like how "fire" and "water" are members of the "elements" set), you should likely being using an Enum. They help avoid errors in many cases, and allow IDEs to assist you in auto-completing names.
In __str__, you could also make use of f-strings to avoid needing to manually concat using +. It would be neater to write:

def __str__(self):
    return f"{self.quality} {self.base} of {self.element}"

That allows for much less noise and allows you to focus more on what you what printed, and less on dealing with opening and closing quotes and +s.
For the sake of context, create_random_item may make more sense as a static method. It makes it clearer what class it's associated with, since then you'd call the method as:

Item.create_random_item()


Get GPS coords with Addresses from CSV I have a script that takes addresses from a CSV, and tries to find the GPS coordinates.

I am still learning Python, so comments as to general formatting (i.e. variable names, function names, etc.) are appreciated, as well as ways to tighten up the code.  (I'm using the csv module and geopy).

The CSV is a single column of addresses (following are dummy addresses) saved as a .csv file:

123 Main Street, Chicago, IL 12345
3214 Overland Drive, Los Angeles, CA 49382
29 NE 9th Street, New York, NY 29384

And my .py file:

import csv
from geopy.geocoders import Nominatim
from geopy.geocoders import GoogleV3

ADDRESS_FILE = "Addresses"

def read_csv(file):
    with open(file,'r') as f:
        reader = csv.reader(f)
        address_list = list(reader)
    return address_list

def get_lat_lon(address):
    geolocator = Nominatim()
    try:
        location = geolocator.geocode(address)
        return location.latitude, location.longitude
    except Exception:
        return 0, 0

def google_lat_lon(address):
    geolocator = GoogleV3(api_key="xxxxx")
    try:
        location = geolocator.geocode(address)
        return location.latitude, location.longitude
    except Exception:
        return 0, 0

def find_coordinates(address):
    google_found = ""
    lat, lon = get_lat_lon(address)
    lat, lon = google_lat_lon(address)
    if lat == 0 and lon == 0:
        lat, lon = google_lat_lon(address)
        google_found = "Note: used Google"
    return lat, lon, google_found

def write_file(info):
    with open(ADDRESS_FILE+" with GPS.csv","w", newline='') as csvfile:
        writer = csv.writer(csvfile, delimiter = ",")
        for line in info:
            writer.writerow(line)

def main():
    addresses = read_csv(ADDRESS_FILE+".csv")
    address_GPS = []
    for address in addresses:
        addr = address[0].replace(u'\xa0', u' ')
        lat, lon, google_found = find_coordinates(addr)
        address_GPS.append((addr,lat,lon, google_found, '=HYPERLINK("https://www.google.com/maps/search/"&B1&","&C1,"Link")'))

    for addr in address_GPS:
        print(addr[:len(addr)-1])
    write_file(address_GPS)

if __name__ == '__main__':
    main()

Preliminary thoughts:

addr = address[0].replace(u'\xa0', u' ') - I have this because for some reason when reading the CSV, \xa0 appears in the string, throwing off the lookup. I think it's due to Encoding? I created the CSV in Excel, File-> Save as .CSV (regular .csv, not the option for Mac, or whatever else).
except Exception: - I think I should be more specific here? That's there in case there's no location found with the address.
The if lat == 0 and lon == 0 is there in case the regular geopy lookup doesn't work, I use their Google Maps option.  But say that doesn't work, and I want to use Bing or edit the address, there's probably a better way than keep on nesting that if lat == 0 and lon == 0 part.
Is it okay to have my main() like I do? Or is it better to keep the lines in main() to primarily be function calls?
Is it wise/good practice to have a return a, b, c, ..., z? Or is it best to return just one or two things per return?

Regarding your questions:

No idea :-)
Yes, you should be more specific. The easiest way to find out what exception it could raise is to run it without the guard and give it an address you know does not exist (I'm pretty sure the address "abdjdiehdbdocoegrvdkdoc" does not exist, for example). Note that you can catch multiple exceptions at once with except (Exception1, Exception2): (the parentheses are required) or with multiple except blocks (if you need to do different things, depending on the exception type).
Yes, your current if lat == 0 and lon == 0 approach does not scale to additional lookup services. It is also currently slightly broken, since you always use Google (you use the function before the check). You could iterate through a list of lookup functions until you find one that returns valid coordinates:

LOOKUP_FUNCS = get_lat_lon, google_lat_lon

def find_coordinates(address, lookup_funcs=LOOKUP_FUNCS):
    for function in lookup_funcs:
        lat, lon = function(address)
        if (lat, lon) != (None, None):
            return lat, lon
    return None, None

Note that I assume here that the functions return None, None, instead of 0, 0, which is arguably a better value for something non-existing. Theoretically, some address could actually be at 0, 0. Note also that csv.writer translates None to "", when writing to a file (see the documentation).

A main that does as much as yours is fine, IMO, if this is the only thing this script does. But you might still want to rename it to add_lat_lon(in_file, out_file) or something like this, to be able to import the actual functionality in some other script.In your new main block (or directly under the if __name__ == '__main__': guard), you could then call that function.
It is perfectly fine to return tuples.

Some additional notes:

You should add docstrings to your functions to document what they do.
csv.writer has a writerows method that allows you to directly pass a list of rows. So you can just do writer.writerows(info).
Instead of print(addr[:len(addr)-1]) you can just do print(addr[:-1])