Reading from a file and connect all data in one big data than to use generators Is there better way to read from a file and connect all data in one big data than to use generators?

At the moment, I do the following:

use generators to read data from files.
use NumPy to pack all files in 3D array.
use pandas to stack it to 2D array so readable for next operations (e.g. plotting).

In my example:

I split reading from file operations generators in 2 modules, and have a third module for reading names (file is not connected with others generators_module). In the last separate file I import that generators_modules, and make arrays with NumPy, then pandas.

In the code of modules I use os.walk to read from files, regex to read only that data which is needed.

In the code of making arrays with NumPy, pandas:

I enter variable parameters needed to get data with generator_modules
I pass from generators to array with code:

xdata=np.array( [(float(line['PRESSURE']), float(line['CURVE'])) for line in dt_cols] )

I have four python files, as below:

module1 (gen_enter):

import re, matplotlib as mpl, matplotlib.pyplot as plt, os, fnmatch

def gen_find(filepat,top):
    for path, dirlist, filelist in os.walk(top):
        for name in fnmatch.filter(filelist,filepat): yield os.path.join(path,name)

def gen_open(filenames):
    for name in filenames:
        if name.endswith(".XL"): yield open(name)
        else: pass

def gen_get(sources):
    for s in sources:
        for item in s:  yield item

def gen_grep(pattern, fileparse):
    for line in fileparse:
        inputlines = line[:].strip().replace(';',' ')
        if pattern.search(inputlines):  yield inputlines
        else: pass

def field_map(dictseq,name,func):
    for d in dictseq:
        d[name] = func(d[name])
        yield d

module2 (gen_returnlines):

from gen_enter import *

def lines_from_dir(filepat, dirname):
    findnames = gen_find(filepat, dirname)
    openfiles = gen_open(findnames)
    getlines = gen_get(openfiles)
    #patlines = gen_grep(pattern, getlines)
    return getlines

module3 (gen_shownames):

import re, matplotlib as mpl, matplotlib.pyplot as plt, os, fnmatch

def gen_shownames(filepat,top):
    for path, dirlist, filelist in os.walk(top):
        for name in fnmatch.filter(filelist,filepat): yield name

The main code:

import numpy as np, pandas as pd, os, fnmatch, re
from pylab import *
from gen_returnlines import *
from gen_shownames import *

def dict_cols(lines):
        groups = (patlines.match(line) for line in getlines)
        tuples  = (group.groups() for group in groups if group)
        colnames = ('PRESSURE','CURVE')
        line = (dict(zip(colnames,t)) for t in tuples)
        line = (field_map(line,"PRESSURE", lambda s: float(s)))
        line = (field_map(line,"CURVE",float))
        return line

dir='C:\\Users\\REDHOOD\\workspace\\Politechnika_python\\\silniki\\files' #note: all files from ../files/
pattern = re.compile(r'(\d{3}\.\d{1})\D*(\d{3}\.\d{1})\D*')
pats = '(\d{3}\.\d{1})\D*(\d{3}\.\d{1})\D*'
patlines = re.compile(pats)

if __name__ == '__main__':
    names = gen_shownames('*', dir)
    getlines = lines_from_dir('*',dir)
    dt_cols = dict_cols(getlines)

xdata = np.array( [(float(line['PRESSURE']), float(line['CURVE'])) for line in dt_cols] )
xrdata = np.reshape(xdata,(17,360,2))

datapanel = pd.Panel(
                  xrdata,
                  items=[k for k in names],
                  )

datapaneldf = datapanel.to_frame()       #pressure na curve -> 360 stopni

Is there better way to read from a file and connect all data in one big data than to use generators?

Whenever possible, using generators is better than collecting data in one big object.
One big object needs a lot of memory at once,
using a generator you can use a pipeline where only the data necessary for the current processing is in memory, and discarded when completed.

I didn't look hard enough in your program if your operation can be fully streamlined.
What I can see is that in the dict_cols method you effectively collect from all the generators:

def dict_cols(lines):
        groups = (patlines.match(line) for line in getlines)
        tuples  = (group.groups() for group in groups if group)

Here, getlines and group are generators,
but you fully consume them.
At this point, it doesn't matter anymore if getlines and group were generators or lists, because everything will be loaded in memory anyway.
I don't know if it's possible to change your processing to avoid fully consuming the generators at this point or later.
It would be great if you could.
But even if you cannot,
it's still good that getlines and group are generators instead of lists,
because maybe in the future you will find a better way to process the data in a more streamlined way without collecting.

Using __name__ properly

The purpose of the if __name__ == '__main__': ... check is to make it possible to load a package without executing code. However, you have this:

if __name__ == '__main__':
    names = gen_shownames('*', dir)
    getlines = lines_from_dir('*',dir)
    dt_cols = dict_cols(getlines)

xdata = np.array( [(float(line['PRESSURE']), float(line['CURVE'])) for line in dt_cols] )
xrdata = np.reshape(xdata,(17,360,2))

This defeats the purpose. It would be better to move everything you do inside the if statement and afterward inside a main method, and call that from the if statement, like this:

def main():
    names = gen_shownames('*', dir)
    getlines = lines_from_dir('*',dir)
    dt_cols = dict_cols(getlines)

    xdata = np.array( [(float(line['PRESSURE']), float(line['CURVE'])) for line in dt_cols] )
    xrdata = np.reshape(xdata,(17,360,2))

    # ... and so on

if __name__ == '__main__':
    main()

Coding style

It seems you're not following PEP8,
the official style guide of Python.
I suggest to give it a good read and follow it.

There shouldn't be multiple statements on one line:

    for name in fnmatch.filter(filelist,filepat): yield os.path.join(path,name)

Break the line after :

    for name in fnmatch.filter(filelist,filepat):
        yield os.path.join(path,name)

Do the same in other places too, always break the line after a :.

The else is pointless here and should be removed:

def gen_open(filenames):
    for name in filenames:
        if name.endswith(".XL"): yield open(name)
        else: pass

I see the same in other methods too, fix it everywhere.

Put a space after method parameters, so instead of:

def field_map(dictseq,name,func):
    for d in dictseq:

Write like this:

def field_map(dictseq, name, func):
    for d in dictseq:

Avoid wildcard imports like this:

from gen_enter import *

This practice limits the ability of IDEs and static analysis tools to check for invalid references.

Instead of this:

dir='C:\\Users\\REDHOOD'

You can write simpler this way:

dir='C:/Users/REDHOOD'


Project Euler problem 2 This is a question about Project Euler number 2, a relatively easy one. The question is:

  Each new term in the Fibonacci sequence is generated by adding the
  previous two terms. By starting with 1 and 2, the first 10 terms will
  be:

  1, 2, 3, 5, 8, 13, 21, 34, 55, 89, ...

  By considering the terms in the Fibonacci sequence whose values do not
  exceed four million, find the sum of the even-valued terms.

My code is pretty simple; I'm interested in other ways of doing it (lamdba expressions for instance).

namespace ProjectEuler
{
    public class Fibonacci
    {
        // find even number fibonacci terms < 4 mil.
        public void FibonacciSequence(int limit)
        {
            var sw = Stopwatch.StartNew();
            int a = 0;
            int b = 1;
            int total = 0;
            int evenTerms = 0;

            while (total < limit)
            {
                total = a + b;
                a = b;
                b = total;
                if (total % 2 == 0)
                {
                    evenTerms += total;
                }
            }
            sw.Stop();
            Console.WriteLine(evenTerms);
            Console.WriteLine(sw.ElapsedMilliseconds);
        }
    }
}

This is the solution I came up with using the Fibonacci succession closed form:

that uses the golden radio:

First we can see that for any even Fibonacci number the sum of all the previous even and odd Fibonacci is equal.

Each even Fibonacci number is preceded by 2 odd Fibonacci numbers.

o o e o o e

1 1 2 3 5 8....

This means that we can transform each of the even Fibonacci in the sum of the 2 previous odd Fibonacci .

So n being any even Fibonacci and f(n) the sum of all even Fibonacci up to n we have:

Also the sum of all Fibonacci numbers up to fib(x) is equal to fib(x+2)-1.

So given any number f we find x by calculating the closest Fibonacci  number fib(x) that is even and less than or equal to f. We then can finally compute  fib(x+2)-1.

The code (this is C++ but it can easily be translated to C#):

long fib ( int i, double golden )
{
    return pow ( golden, i ) / sqrt ( 5 ) + 0.5 ;
}

long findSumEvenFibUpto ( int number )
{
    const double golden = ( 1 + sqrt ( 5 ) ) / 2 ;
    long nearest = round ( log ( sqrt ( 5 ) * number ) / log ( golden ) );

    int fibo = 0 ;
    while ( ( fibo = fib( nearest -- , golden ) ) % 2 || fibo > number ) ;

    return ( fib ( nearest + 3 , golden ) - 1 ) / 2 ;
}


Project Euler # 37 Truncatable primes in Python The number 3797 has an interesting property. Being prime itself, it is possible to continuously remove digits from left to right, and remain prime at each stage: 3797, 797, 97, and 7. Similarly we can work from right to left: 3797, 379, 37, and 3.
Find the sum of the only eleven primes that are both truncatable from left to right and right to left.
NOTE: 2, 3, 5, and 7 are not considered to be truncatable primes.

Here's my implementation in Python, it's very slow(takes like 10 secs to show results. How to make it faster, more efficient?
def is_prime(number):
    """returns True for a prime number, False otherwise."""
    if number == 1:
        return False
    factor = 2
    while factor * factor <= number:
        if number % factor == 0:
            return False
        factor += 1
    return True

def get_truncatable(n):
    """returns truncatable numbers within range n."""
    for number in range(9, n, 2):
        if is_prime(number):
            check = 0
            for index in range(-1, -len(str(number)), -1):
                less_right = str(number)[:index]
                if not is_prime(int(less_right)):
                    check += 1
            if check == 0:
                for index in range(1, len(str(number))):
                    less_left = str(number)[index:]
                    if not is_prime(int(less_left)):
                        check += 1
                if check == 0:
                    yield number

if __name__ == '__main__':
    print(sum(list(get_truncatable(1000000))))

You know that a prime other than 2 and 5 ends in 1, 3, 7 or 9. What does that tell you about the digits of x if it is right truncatable? It tells you that all the digits after the first must be 1, 3, 7 or 9.

On the other hand, if you have a four digit left-truncatable number, then the last three digits are also left truncatable. So the find the n+1 digit both left and right truncatable numbers, you take an n digit one which starts with 1, 3, 7 or 9, and prepend any of the digits 1 to 9, and check that the result is both a prime and right-truncatable.


Show days of single week I'm currently in the process of creating a online work schedule for internal use. For this, the client needs to show the days of a single week.

I pass a week number and a year.
From that I get the first day of that week, by using strtotime() passing an argument in the format [year]-W[weekno]-[dayofweek]. Example: strtotime('2011-48W-1');

With that function I create my table as mentioned earlier:

            <?php for($i = 1; $i <= 7; $i++): ?>

            <?php echo $dage[$i] . " ". date("j/m", strtotime($year.'-W'.$week.'-'.$i)); ?>

            <?php endfor; ?>

            <?php for($i = 1; $i <= 7; $i++): ?>

            <td<?php if($i == 7) { echo ' class="last"'; } ?>>

                <?php

                $day = date("Y-m-d", strtotime($year.'-W'.$week.'-'.$i));

                if(isset($entries[$day])):

                foreach($entries[$day] as $entry):

                ?>

                <div class="workEntry" title="<?php echo $entry->note; ?>">

                        <?php echo date("H:i", strtotime($entry->workStart)); ?>

                        <?php if($entry->workStart != "0000-00-00 00:00:00") { echo " - ".date("H:i", strtotime($entry->workEnd)); } ?>

                    <?php if($entry->userId == 0): ?>

                    Unspecifed

                    <?php else: ?>

                    <?php echo $entry->name; ?>, <?php echo $entry->pgroup; ?>

                    <?php endif; ?>

                <?php endforeach;

                endif; ?>

            <?php endfor; ?>

Well, it works, but I'm pretty sure that I'm reinventing the wheel, and doing a pretty bad job at it too. I'd like to know what you think and to hear any suggestions.

Three minor notes:

From Steve McConnell, Code Complete 2nd Edition, 31.2 Layout Techniques, p737:

  Using blank lines is a way to indicate how a program is organized. You can use them
  to divide groups of related statements into paragraphs, to separate routines from one
  another, and to highlight comments.

It does not make sense if all groups contain only one line.
strtotime($year.'-W'.$week.'-'.$i) is used and duplicated multiple times in the code. You could extract it out to a function to remove the duplication.
If I'm right you are iterating through the days of the week with this loop:

<?php for($i = 1; $i <= 7; $i++): ?>

Renaming $i to $weekday or something similar would make it obvious and help readers/maintainers a lot.


concise way to build an object from an array of objects The following code is working as expected but I'm wondering if there's a way to use Object.assign, or some other approach, to build the derivedData object so that it doesn't need to be initialized as an empty object and then populated in a subsequent step.
const data = [
  {"period": "2021-05-01", "quantity": "28"},
  {"period": "2021-06-01", "quantity": "42"},
  {"period": "2021-07-01", "quantity": "66"}
];

const derivedData = {};
data.forEach(o => {
  derivedData[o.period] = o.quantity
});

console.log(data);
console.log(derivedData);

Expected output of derivedData:
{
  "2021-05-01": "28",
  "2021-06-01": "42",
  "2021-07-01": "66"
}

While it technically still would initialize it as an empty object, one could build derivedData with one call to Array.prototype.reduce(). It is similar to forEach() except that the first argument of the callback function is an accumulator variable, that needs to be returned at the end of the function. See the snippet below for a demonstration.

const data = [
  {"period": "2021-05-01", "quantity": "28"},
  {"period": "2021-06-01", "quantity": "42"},
  {"period": "2021-07-01", "quantity": "66"}
];

const derivedData = data.reduce((accumulator, o) => {
  accumulator[o.period] = o.quantity;
  return accumulator;
}, {});

console.log('derivedData: ', derivedData);

You may find these functional exercises interesting.
Good job using const for variables that don't need to be re-assigned, and using common spacing conventions for modern JavaScript.


Detecting collision of two divs I have two divs as seen in this JSFiddle. To recreate the problem, keep clicking on the Gray container till both divisions touch each other.

The purpose is to execute a certain function once the green div touches the red one. The problem is that, this step callback as seen from the following code and demo, gets called multiple times. It is not something that I want to happen.

One method that would fix the problem is to keep a certain flag variable which turns True once step is executed. But I'm not sure that's enough as there needs to be a way to break out of the step callback.

I want to know:

If there is a better/efficient way to detect collision of two divs than using jQuery Collision.
How to stop the step callback after its executed once.

HTML:

JavaScript:

$(document).ready(function () {
    var hit_list;
    $(".container").click(function () {

        $(".menu").stop().animate({
            left: "+=100px"
        }, {
           duration: 300,
           complete: function () {
               $(".menu").animate({
                   left: "0"
               }, 800);
           },
           step: function(){
               //Test for collision
               hit_list = $(".menu").collision(".test");
               if (hit_list.length != 0) {
                   $(".container").append("Welcome Earthling");
               }
           }
         });
     });
 });

In response to not triggering the message repeatedly, I would store the previous check for a collision so you know if a collision has started or ended.

If there is a collision but your previous check didn't show a collision, then you have a new collision and you can trigger the event. However, if the previous check did show a collision, don't fire the callback since it has already been fired.

I would also recommend using custom jQuery events. This allows you to separate out the callback from the animation code.

Below I've adapted your code to trigger a custom event when you detect a collision has started or ended. Here's the updated javascript (also available as a modification of your fiddle):

$(document).ready(function () {

    $(".menu").data("collision", false);
    $(".menu").on("collision_start", function () {
        $(".container").append("Welcome Earthling");
    });

    $(".container").click(function () {

        $(".menu").stop().animate({
            left: "+=100px"
        }, {
            duration: 300,
            complete: function () {
                $(".menu").animate({
                    left: "0"
                }, 800);
            },
            step: function () {
                //Test for collision
                var hit_list = $(this).collision(".test");
                var current_collision = hit_list.length != 0;

                // compare this to what we last observed
                var changed_collision = current_collision != $(this).data("collision");

                if (changed_collision) {
                    // save this new observation for future comparisons
                    $(this).data("collision", current_collision);

                    // trigger an event
                    $(this).trigger("collision_" + (current_collision ? "start" : "stop"));
                }
            }
        });
    });
});


Dueling beings on a grid I'm remaking an old program of mine in Clojure. It involves "Beings" (generic entities capable of attacking and breeding) let loose in a confined space.

I've noticed a pattern of taking a record, creating a modified version of one of its fields, then "overwriting" the field.

It generally seems to look like:

(defn map-pts [p1 p2 f]
  (let [[x1 y1] p1
        [x2 y2] p2]
    [(f x1 x2) (f y1 y2)]))

(defn add-pts [p1 p2]
  (map-pts p1 p2 +))

(defrecord Being [position, health])

(defn hurt [being damage]
  (let [newHealth (- (:health being) damage)]           ;<<<
    (assoc being :health newHealth)))                   ;<<<

(defn move-by2 [being x y]
  (let [new-position (add-pts (:position being) [x y])] ; <<<
    (assoc being :position new-position)))              ; <<<

(I'm only including the first 2 functions so it's complete).

I decided to write a function to try and limit the boilerplate.

(defn update-with [m k k1->k2]
  (let [oldValue (get m k)]
    (assoc m k (k1->k2 oldValue))))

which can be used like:

(defn hurt2 [being damage]
  (update-with being :health
                 #(- % damage)))

(defn move-by [being x y]
  (g/update-with being :position
                 #(add-pts % [x y])))

which I find to be significantly cleaner.

What I want comments on:

Am I thinking about this is the right way? Is this something I should be worrying about, or is this an xy problem?
Is k1->k2 an appropriate name? It seems like a very self-documenting way of indicating what type of function is needed, but is also "Hungarian", which I know is frowned upon.
This seems like something that would be included in the langage given its simplicity. Does this function already exist?

An example of how it will be used (non-functioning code, since some components aren't yet defined):

(def beings ['(all the beings in the world)])

(map (fn [being]
       (do
         (if (is-contacting-other? being beings)
           (hurt being 20)                        ; <<---
           being))))

In reality, I'll be giving the job of calling hurt to a helper function attack that will decide how much damage to do. This same loop will also handle other things, so it will be a little more complex. This is just a stripped-down example.

I don't think trying to cut down boilerplate like that is too much of an issue.
Normally I would have no problem with using something->somethingelse for the name of a function that converts from something to something else.  I'm not sure in this case though.  For one thing, I'd assume k means key.  k1->k2 kind of suggests it's acting on the key, but it's not.  In this case I'd probably just follow the example of the stdlib and call it f.  Makes it obvious it is the function that does the updating.  (Semi-related: you might want to check out the sample chapter of elements of clojure - it goes into naming things in clojure in some depth)
If you're running clojure 1.7+, you've got the update function that does this.


Object Oriented JavaScript optimization This is a simple script that I use for wrapping a selected word in a textarea field.  I'm looking to:

make sure my logic is valid/see what others have done in a relative context.
request improvements to the OO approach I'm going for.

Formatter = {};

Formatter.getSelection = function(event) {
  var text  = document.getElementById("mass_message");
  var sPos  = text.selectionStart;
  var ePos  = text.selectionEnd;
  var front = (text.value).substring(0, sPos);
  var sel   = (text.value).substring(sPos, ePos);
  var back  = (text.value).substring(ePos, text.value.length);
  format    = event.data.format;

  var print = Formatter.setString(text, format, front, back, sel);
}

Formatter.setString = function(text, format, front, back, sel) {
  text.value = front + "<" + format + ">" + sel + "</" + format + ">" + back;
}

$(document).ready(function(){
  $( '#bolden' ).on('click', { format: "b" }, Formatter.getSelection);
  $( '#italicize' ).on('click', { format: "i" }, Formatter.getSelection);
});

I'm appreciative of any suggestions and am happy to answer questions about the code.

One comment I might suggest is your hard reference to mass_message, consider passing that in as part of your event data, as it makes the code more re-usable (Several instances of your editor on a page?)

A thought to consider is to swap your hard b and i tags to instead define css classes to be used? This will give you code re-use, if you had bold and italic defined in a css, you could use the above code you have to simply change the class on click instead, allowing you to do more jazzy stuff, borders and such, or be very clever and have specific things you define, maybe floats and margins/padding etc. You could use span as a first try and set the class of the span equal to the specified format :)


The beginnings of a bootloader using GNU-based tools This is a personal project of mine.  I am currently in the research phase of developing my own bootloader that will be used to load a single task operating system. I am far away from the final goal, but have been trying to document my process as a reference. The content of my code documentation and code can be found at Bootloader 101.

If possible I would appreciate feedback. The wiki page is open to be edited, and if you feel like you can contribute please feel free to do so. I apologize beforehand for referencing you to another page.  The format of the content is important.

First piece of code:

   # Author: Matthew Hoggan
   # Date Created: Tuesday, Mar 6, 2012
   .code16                        # Tell assembler to work in 16 bit mode (directive)
   .section .text
   .globl _start                  # Help linker find start of program
   _start:
       movb $0x0e,     %ah        # Function to print a character to the screen
       movb $0x00,     %bh        # Indicate the page number
       movb $0x07,     %bl        # Text attribute
       mov  $'A',      %al        # Move data into low nibble
       int  $0x10                 # Video Service Request to Bios
   _hang:
       jmp  _hang
       .end

   /* linker.ld */
   SECTIONS
   {
       /* Set location counter to 0x7c00 */
       . = 0x7c00;
       .text : { *(.text) }
       .data : { *(.data) }
       .other :
       {
            FILL(0x000000)
            . = 0x1F2;
            BYTE(0x55)
            BYTE(0xAA)
       }
   }

Second piece of code:

   # Author: Susam Pal <http://susam.in/>
   .code16                        # Tell the assembler to use 16 bit instructions
   .section .text
   .globl _start
   _start:                        # Tell linker where entry point into program is
       mov $0xb800, %ax           # 0xb800 is the start address of character buffer
       mov %ax, %ds               # Move that address into the data segment
       movb $'B', %ds:0x00        # Write data directly to that buffer at 0
       movb $0x1e, %ds:0x01       # Write text attributes for the text right after
   idle:
       jmp idle

   /* linker.ld */
   SECTIONS
   {
       /* Set location counter to 0x7c00 */
       . = 0x7c00;
       .text : { *(.text) }
       .data : { *(.data) }
       .other :
       {
            FILL(0x000000)
            . = 0x1ED;
            BYTE(0x55)
            BYTE(0xAA)
       }
   }

Your code is well-commented. Other assembly answers on this site mention the importance of good comments: and yours are good.
An infinite loop at the end of the program is strange; instead I would have expected a ret or retf statement to return to the O/S shell which launched this program; but maybe that's different/excusable for what may in future evolve to become a boot-loader.
Writing to memory at 0xb8000 may or may not work depending on the current video mode. Here is a thread which discusses whether it's safe to assume 0xb8000 on computer startup (perhaps it is, but there are alternatives)
You code would be more compact (perhaps less readable) if you initialized entire words, instead of separate instructions for each byte; for example in the first program you could initialize ax and bx; and in the second program you could write 0x1E42 into the first word of video memory.
Using the 2nd method you'll probably start writing whole strings (not single characters) to the screen; the lods/stos/movs opcodes are useful for that. For example, you could do:

mov $0x1e, %ah # Write text attributes for the text right after
loop:
lodsb          # load byte from ds:si and increment si
cmp $0x00, %al # test for end-of-string
jz done
stosw          # store word (not byte) to es:di and increment di
jmp loop       # loop back to load-and-then-store next byte of the string
done:

For this kind of reason it's unconventional of you to use the ds register to point to video memory; I would have expected you to use the es register instead.
According to the GNU Assembler manual if you write %ds:0x00 then it will emit the ds prefix, which is unnecessary (I think that ds is the default segment for memory access); perhaps (based on the manual, I don't know) the syntax without the segment prefix is something like just 0x00 or perhaps 0x00(,1).

(I'm unable to give you feedback on the linker.ld section).


Squaring an integer by repetitive addition I've written two functions:

is_integer(), which tries to get a positive or a negative integer as input and returns the same value.
square(), which takes this returned value as parameter and returns square of the same.

The code in square() uses a for-loop that squares the integer by 'repetitive addition.' Finally, the script prints out the square of the integer returned by square().

Please review my code, point out errors of any kind and make suggestions.

# Squares an integer by repetitive addition
# Uses a for-loop
def square(integer):
    result = 0

    for element in range(1, abs(integer)+1):
        result += abs(integer)
    return 'Square of ' + str(integer) + ' is ' + str(result)

# Makes sure the input is an integer
def is_integer():
    number = raw_input('Enter an integer: ')

    if number.isdigit():
        number = int(number)
        return number
    elif number[0] == '-' and number[1:].isdigit():
        number = int(number)
        return number
    else:
        print '%r is not an integer' % number
        print 'To try again, press Enter key'
        print 'Otherwise, press Ctrl+C keys to abort'
        raw_input('>>> ')

        return is_integer()

# Square() takes as parameter the value returned by is_digit()
# Print the result returned by square()
print square(is_integer())

Improving square

There are several bad practices in square:

Multiple calls to abs(integer) when it could be calculated just once and reused
Instead of range(1, x + 1) you could write range(x)
When you don't need to use the loop variable element for anything, a common convention is to name it _ instead
Converting integer and result to string is tedious. It would better to use a formatted string expression

Making the appropriate changes the method becomes:

def square(integer):
    result = 0
    abs_integer = abs(integer)

    for _ in range(abs_integer):
        result += abs_integer
    return 'Square of {} is {}'.format(integer, result)

Improving is_integer

In addition to what @Jamal already pointed out, the method has other problems too:

The way you check if the user input is numeric is overly tedious
Naming the input as "number" is not great, because it might be text

A better way to write it (also fixing the name and making it non-recursive):

def prompt_integer():
    while True:
        text = raw_input('Enter an integer: ')

        try:
            return int(text)
        except ValueError:
            print '%r is not an integer' % text
            print 'To try again, press Enter key'
            print 'Otherwise, press Ctrl+C keys to abort'
            raw_input('>>> ')

Why Python 2.7 ?

You're using Python 2.7.
Note that there are no more planned releases for Python 2.7 beyond 2015 June.
Consider migrating to Python 3, it's the future.


Ruby function to fetch, filter, and generate data I'm currently writing a script and I decided to run cane over it to look for some issues, and the following method was highlighted. I've done my best to cut it back, and at this point I'm inclined to leave it as-is, however cane is still giving it a pretty high rating of 23 (15 is the point it starts complaining) so I wanted to get some more input. Other than perhaps splitting fetching and generating does anyone have any ideas?

def self.generate(template, output_file, company)
  puts 'Fetching data...'
  @data = Provider::GenericProvider.create_provider(company['type'].to_sym, company).data

  valid_keys = @data.keys.sort {|a,b| -(a <=> b)}[0,10]
  @data = @data.select {|k, _| valid_keys.include? k}

  valid_keys = @data.keys.sort {|a,b| -(a <=> b)}[0,2]

  loop do
    break if :done == choose {|menu| timesheet_menu valid_keys, menu}
  end

  puts 'Generating output...'
  # Used as a result of the ERB binding
  #noinspection RubyUnusedLocalVariable
  data = @data.select {|k, _| valid_keys.include? k}
  File.write(output_file, ERB.new(File.read(template), nil, '-').result(binding))
end

EDIT:

I just split it, and ended up with the below function, which still scores 19 due to the various lambdas used to control sorting and filtering the data despite being pretty easy to follow still:

def self.fetch_data(company)
  data = Provider::GenericProvider.create_provider(company['type'].to_sym, company).data

  valid_keys = data.keys.sort {|a,b| -(a <=> b)}[0,10]
  data = data.select {|k, _| valid_keys.include? k}

  valid_keys = data.keys.sort {|a,b| -(a <=> b)}[0,2]

  loop do
    break if :done == choose {|menu| timesheet_menu data, valid_keys, menu}
  end

  data.select {|k, _| valid_keys.include? k}
end

Matthew, it appears to me that you are only making use of the two elements of data whose keys are the top two in the sort.  Please correct me if I am wrong.  If that is the case, I believe your code (after "edit") can be simplified to this:

  def self.fetch_data(company)
    data = Provider::GenericProvider.create_provider(company['type'].to_sym, company).data
    data = data.sort {|(k1,_),(k2,_)| -(k1<=>k2)}.first(2)
    loop do
      break if :done == choose {|menu| timesheet_menu data, data.map(&:first), menu}
    end
    Hash[data]
  end

Note that after the sort, data is an array of two 2-tuples, each corresponding to a key-value pair from the original hash.  If I misunderstood what you are doing, and you do need to pull out the 10 elements of the hash corresponding to the 10 highest-ranking keys, just change the parameter for first from 2 to 10, then extract the first two elements when you need them (i.e., data.first(2)).  You can then retrieve the key values from these 2-tuples or convert data.first(2) to a hash for the return value, as I have done with data.

Edit: Ah, valid_keys is modified by choose. Consider this as a further possibility:

 data = Hash[data.sort{|(a, _), (b, _)| -(a <=> b)}.first(10)]

 loop do
   break if (keys_chosen = choose {|menu| timesheet_menu data, data.keys.first(2), menu})
 end

 data.select {|k, _| keys_chosen.include? k}


Conversion of Haskell CBOR decoding code into Rust I was interested in converting the following Haskell code, that does decoding of CBOR into Rust:

module Main where

import Data.Word
import Data.Bits
import Control.Monad.Trans.Except

data Value = I Int deriving (Show)

read_int xs size =
    f xs size 0
    where
        f _  0 acc     = return (I acc)
        f [] _ acc     = throwE ()
        f (x:xs) n acc = f xs (n - 1) (acc `shiftL` 8 .|. fromIntegral x)

cbor_decode (x:xs)
    | x >= 0x00 && x <= 0x17 = return (I (fromIntegral x))
    | x >= 0x18 && x <= 0x1b = let v = fromIntegral x - 0x17 in read_int xs v
    | otherwise = throwE ()

main :: IO ()
main = do
    let x = [0x1a, 1, 0, 0] :: [Word8]
    let y = runExcept (cbor_decode x) :: Either () Value
    putStrLn $ show $ y

I ended up with the following:

use Value::*;

#[derive(Debug)]
enum Value {
    I(i64),
}

fn read_int(buf: &[u8], size: usize) -> Result<Value, ()> {
    fn f(buf: &[u8], size: usize, acc: i64) -> Result<Value, ()> {
        if size == 0 {
            Ok(I(acc))
        } else if buf.len() == 0 {
            Err(())
        } else {
            f(&buf[1..], size - 1, acc << 8 | buf[0] as i64)
        }
    }
    f(buf, size, 0)
}

fn cbor_decode(buf: &[u8]) -> Result<Value, ()> {
    match buf[0] {
        0x00 ... 0x17 => Ok(I(buf[0] as i64)),
        0x18 ... 0x1b => read_int(&buf[1..], (buf[0] - 0x17) as usize),
        _             => Err(()),
    }
}

fn main() {
    let x = vec!(0x1a, 1, 0, 0);
    let y = cbor_decode(&x);
    println!("{:?}", y);

}

Is this considered good Rust?

I'd expect some kind of error type to describe what the problem was. There's at least two types of error I see, but the user cannot distinguish between them.
Rust does not perform tail-call optimization, so it's usually better to write things iteratively, especially when there's no fixed bound.
Using is_empty is more immediately obvious as to the desired behavior.
It seems very strange to read arbitrary integer lengths. Since the size is a usize, that means you could read an integer that takes 4 billion bytes or more! This is especially strange considering you can only return an i64.
Would encode the unit of size somehow. The easiest is to add _in_bytes to the argument name.
The vec! macro idiomatically uses square brackets to look like arrays.
There's no reason to allocate a vector here anyway; an array works fine.

use Value::*;

#[derive(Debug)]
enum Value {
    I(i64),
}

#[derive(Debug)]
enum Error {
    NotEnoughData,
    InvalidInteger,
}

fn read_int(mut buf: &[u8], size_in_bytes: usize) -> Result<Value, Error> {
    let mut acc = 0;

    for _ in 0..size_in_bytes {
        if buf.is_empty() { return Err(Error::NotEnoughData) }
        acc <<= 8;
        acc |= buf[0] as i64;
        buf = &buf[1..];
    }

    Ok(I(acc))
}

fn cbor_decode(buf: &[u8]) -> Result<Value, Error> {
    match buf[0] {
        0x00 ... 0x17 => Ok(I(buf[0] as i64)),
        0x18 ... 0x1b => read_int(&buf[1..], (buf[0] - 0x17) as usize),
        _             => Err(Error::InvalidInteger),
    }
}

fn main() {
    let x = [0x1a, 1, 0, 0];
    let y = cbor_decode(&x);
    println!("{:?}", y);
}

A good programming practice is to not reimplement things for no reason. To that end, there's already crates for reading numbers out of byte slices:

extern crate byteorder;

use byteorder::{ByteOrder, BigEndian};

fn read_int(buf: &[u8], size_in_bytes: usize) -> Result<Value, Error> {
    if buf.len() < size_in_bytes {
        Err(Error::NotEnoughData)
    } else {
        Ok(I(BigEndian::read_int(buf, size_in_bytes)))
    }
}

Even better, there's already crates for reading and writing CBOR data, such as serde_cbor. This plugs in to the de facto Rust serialization library, allowing you to operate at a higher level, defining structs that can be transformed to and from CBOR automatically.


Marking duplicate entries in a numpy array as True So the problem is

Find the duplicate entries (2nd occurrence onwards) in the given numpy
array and mark them as True. First time occurrences should be False.

And my solution is:
import numpy as np

def duplicates(a):
    uniques = np.unique(a)
    result = []
    for i in a:
        if i not in uniques:
            result.append(True)
        else:
            result.append(False)
            uniques = np.delete(uniques, np.where(uniques == i))
    return result

np.random.seed(100)
a = np.random.randint(0, 5, 10)
print('Array: ', a)
print(duplicates(a))

What improvments can i make in this program?

As mentioned in the comments of another answer, you should use sets for membership tests whenever possible. Combinig this with building up uniques instead of cutting it down, we get this really simple implementation:
def duplicates(numbers):
    uniques = set()
    result = []

    for num in numbers:
        result.append(num in uniques)
        uniques.add(num)

    return result

This implementation of duplicates is concise, easy to read and runs in O(n) instead of O(n^2), which is a major difference. One-liners are a fun exercise that can teach you some interesting aspects of Python, and I understand their appeal, but they're often not the best way to go. It's not worth sacrificing readability and performance for the sake of writing less lines of code.

EDIT: Performance testing
As requested in the comments I tested the performance of three different approaches:
def duplicates_set(numbers):
    uniques = set()
    result = []

    for num in numbers:
        result.append(num in uniques)
        uniques.add(num)

    return result

def duplicates_list(numbers):
    result, uniques = [], []
    for elem in numbers:
        if elem in uniques:
            result.append(True)
        else:
            result.append(False)
            uniques.append(elem)

    return result

def duplicates_oneliner(numbers):
    return [elem in numbers[:i] for i, elem in enumerate(numbers)]

Note that the following approach for duplicates_list should be avoided in any case, since it checks for elem membership in uniques twice.
def duplicates_list(numbers):
    result, uniques = [], []
    for elem in numbers:
        result.append(elem in uniques)
        if elem not in uniques:
            uniques.append(elem)

    return result

Here is my code for performance testing using the timeit module. All the provided outputs were produced by this code.
num_range: (int, int)
random_nums: int
executions: int

np.random.seed(100)
a = np.random.randint(*num_range, random_nums)

from timeit import timeit

print(f"{executions} executions with {random_nums} random numbers in range {num_range}")
print("duplicates_set:", "\t" * 2, timeit(stmt=lambda: duplicates_set(a), number=executions))
print("duplicates_list:", "\t" * 2, timeit(stmt=lambda: duplicates_list(a), number=executions))
print("duplicates_oneliner:\t", timeit(stmt=lambda: duplicates_oneliner(a), number=executions))

Starting out with 100_000 executions with the parameters from the original question we get this result:
100000 executions with 10 random numbers in range (0, 5)
duplicates_set:          0.22261620000000001
duplicates_list:         0.18974730000000006
duplicates_oneliner:     2.9783912

We can clearly see that the list implementation performs the best for a small amount of numbers and a small range of random numbers. The one-liner is already at a huge disadvantage.
Upping the length of the random number array to 1_000, we get this result:
1000 executions with 1000 random numbers in range (0, 5)
duplicates_set:          0.1603748
duplicates_list:         0.13001250000000003
duplicates_oneliner:     3.1837349

It seems the length of the array alone does not really affect the relative performance of duplicates_set and duplicates_list. As long as the range of numbers is small, the list comes out on top. Relative performance stayed the same up until 1_000_000 elements, which makes sense since the size of uniques is capped by the range of numbers.
Once we start increasing num_range, the set implementation really starts to shine, since it simply does not care about the size of uniques when it comes to runtime:
1000 executions with 1000 random numbers in range (0, 10)
duplicates_set:          0.15972010000000003
duplicates_list:         0.15932980000000002
duplicates_oneliner:     3.1950098

For a range of (0, 10), set and list implementation perform about the same.
At a range of (0, 1_000) the list implementation is already outperformed by the one-liner:
1000 executions with 1000 random numbers in range (0, 1000)
duplicates_set:          0.15644170000000002
duplicates_list:         3.4406095
duplicates_oneliner:     3.1854200999999995

The one-liner however is immediately out of the competition once the input gets larger, even at small number ranges:
10 executions with 100000 random numbers in range (0, 10)
duplicates_set:          0.1576303
duplicates_list:         0.24761889999999998
duplicates_oneliner:     30.1282213

The final result is exactly what would be expected:

duplicates_list really starts to struggle once the number of unique numbers goes up, since if elem in uniques gets increasingly expensive.
duplicates_oneliner completely breaks down with long input lists, since elem in numbers[:i] gets expensive really fast.
duplicates_set is only outperformed at a low number of unique numbers, but at scale it outperforms both other implementations by a huge margin, since if elem in uniques can be checked in constant time.


Calculating compass direction I have a heading, which is an integer and from it I would like to figure out the compass direction (North, North-East,...) and return the appropriate icon. I feel like I'm probably already at the cleanest solution but I would love to be proven wrong.

public GetHeadingImage(string iconName, int heading){
        if (IsNorthHeading(heading))
            return iconName  + "n" + ICON_FILE_EXTENTION;
        if (IsNorthEastHeading(heading))
            return iconName + "ne" + ICON_FILE_EXTENTION;
        if (IsEastHeading(heading))
            return iconName + "e" + ICON_FILE_EXTENTION;
        /*A bunch more lines*/
}
private bool IsNorthHeading(int heading)
{
    return heading < 23 || heading > 337;
}
/* A bunch more extracted test methods*/

A simple solution is a good solution:

public GetHeadingImage(string iconName, int heading){
    var directions = new string[] {
        "n", "ne", "e", "se", "s", "sw", "w", "nw", "n"
    };

    var index = (heading + 23) / 45;
    return iconName + directions[index] + ICON_FILE_EXTENSION;
}


Parse webpage and save fetched images to a directory I had a task for a programmer position which I failed. I am a newbie programmer and I accept that. The only problem is that employer never told me what the actual problem with the code is. So maybe community will be able to give few hints on how to improve it.

The task was to write code which would parse a given webpage, fetch all the images and save it to given directory. Webpage address and directory are command line parameters. Performance was a critical issue for this task.

require 'open-uri'
require 'nokogiri'

class Grab
    def runner(url, path)
        threads = []
        doc = Nokogiri::HTML(open("http://#{url}"))
        img_srcs = doc.css('img').map{ |i| i['src'] }.uniq
        img_srcs = rel_to_abs(url, img_srcs)
        img_srcs.each do |img_src|
            threads << Thread.new(img_src) do
                name = img_src.match(/^http:\/\/.*\/(.*)$/)[1]
                image = fetch img_src
                save(image, name, path)
            end
        end
        threads.each{ |thread| thread.join }
    end

    def fetch(img_src)
        puts "Fetching #{img_src}\n"
        image = open(img_src)
    end

    def save(image, name, path)
        File.open("#{path}/#{name}", "wb"){ |file| file.write(image.read) }
    end

    def rel_to_abs(url, img_srcs)
        img_srcs.each_with_index do |img_src, index|
          img_srcs[index] = "http://#{url}/#{img_src}" unless   img_src.match(/http:\/\//)
        end
        img_srcs
    end
end

Few minor issues:

It will be cool if you support URL parameter with protocol (HTTP/HTTPS):

doc = Nokogiri::HTML(open("http://#{url}"))

For URL manipulations you can use standard tools - URI::HTTP/URI:HTTPS instead regexps:

name = img_src.match(/^http:\/\/.*\/(.*)$/)
img_srcs[index] = "http://#{url}/#{img_src}" unless   img_src.match(/http:\/\//)

This is a little unsecure operation, you should escape name:

File.open("#{path}/#{name}", "wb")

You ignore links with HTTPS protocol:

img_src.match(/http:\/\//)

This is not the Ruby way:

img_srcs.each_with_index do |img_src, index|
  img_srcs[index] = "http://#{url}/#{img_src}" unless   img_src.match(/http:\/\//)
end
img_srcs

A little simpler

img_srcs.map do |src|
  src.match(/http:\/\//) ? "http://#{url}/#{src}" : src
end

You load page uneffiсiently:

image = open(img_src)

So for every image temp file created.  This may not be an efficient way.

open("http://stackoverflow.com/questions/1878891/how-to-load-a-web-page-and-search-for-a-word-in-ruby").class
=> Tempfile


Re-positioning controls on a form The following piece of code is used to re-position controls on a form considering different options.

Once you have multiple controls this becomes quite messy. I would appreciate your improvement suggestions.

private void dlgOptionsForm_Load(object sender, System.EventArgs e)
    {

        if (_hideAccountingOption)
        {
            grpAccountingOption.Visible = false;
            Height -= grpAccountingOption.Height;

            grpPeriod.Top -= (grpAccountingOption.Height + 8);
            grpPrintOptions.Top -= (grpAccountingOption.Height + 8);
            grpInvoiceInformation.Top -= (grpAccountingOption.Height + 8);
            grpHND.Top -= (grpAccountingOption.Height + 8);
            chkPrint.Checked = false;
            grpArchiveDocument.Top -= (grpAccountingOption.Height + 8);
            grpBalanceControlList.Top -= (grpAccountingOption.Height + 8);
        }

        if (_hidePeriod)
        {
            grpPeriod.Visible = false;
            Height -= grpPeriod.Height;

            grpPrintOptions.Top -= (grpPeriod.Height + 8);
            grpInvoiceInformation.Top -= (grpPeriod.Height + 8);
            grpHND.Top -= (grpPeriod.Height + 8);
            chkPrint.Checked = false;
            grpArchiveDocument.Top -= (grpPeriod.Height + 8);
            grpBalanceControlList.Top -= (grpPeriod.Height + 8);
        }

        if (_hidePrintOptions)
        {
            grpPrintOptions.Visible = false;
            this.Height -= grpPrintOptions.Height;
            grpInvoiceInformation.Top -= (grpPrintOptions.Height + 8);
            grpHND.Top -= (grpPrintOptions.Height + 8);
            chkPrint.Checked = false;
            grpArchiveDocument.Top -= (grpPrintOptions.Height + 8);
            grpBalanceControlList.Top -= (grpPrintOptions.Height + 8);
        }

        ....
    }

Solution 1:

If all of your grp* inherits of the same object (say Control), what you can do is:

private List<Control> _groups = new List<Control>();

.ctor()
{
   // Populate _groups
   _groups.Add(grpPeriod);
   _groups.Add(grpPrintOptions);
   _groups.Add(grpInvoiceInformation);
   _groups.Add(grpHND);
   _groups.Add(grpArchiveDocument);
   _groups.Add(grpBalanceControlList);
}

Now you method becomes:

private void dlgOptionsForm_Load(object sender, System.EventArgs e)
{
    if (_hideAccountingOption)
    {
        Adjust(grpAccountingOption);
    }
    if (_hidePeriod)
    {
        Adjust(grpPeriod);
    }

    if (_hidePrintOptions)
    {
        Adjust(grpPrintOptions);
    }

    ....
}

private void Adjust(Control control)
{
    control.Visible = false;
    this.Height -= control.Height;
    chkPrint.Checked = false;

    foreach (var item in _groups)
    {
        if(control != item)
            item.Top -= (control.Height + 8);
    }
}

EDIT

Solution 2:

Dictionary<Func<bool>, Control> _groups= new Dictionary<Func<bool>, Control>();

.ctor()
{
   // Populate _groups
   _groups.Add(() => _hidePeriod, grpPeriod);
   _groups.Add(() => _hideInvoiceInformation, grpInvoiceInformation);
   _groups.Add(() => /* your bool variable */, /* the associated control */);
   // ...
}

private void dlgOptionsForm_Load(object sender, System.EventArgs e)
{
    foreach (var item in _groups)
    {
        if (item.Key()) // Evaluate the boolean variable
            Adjust(item.Value);
    }
}

private void Adjust(Control control)
{
    control.Visible = false;
    this.Height -= control.Height;
    chkPrint.Checked = false;

    foreach (var item in _groups)
    {
        if(control != item)
            item.Top -= (control.Height + 8);
    }
}


Maintaining row height while inserting I have a subroutine which re-formats cells after a heading is inserted into the document.

Most cells are 12.75 points high, whereas the headings are 15 points high.

When a heading (single line) is inserted, it pushes all of the other cells down, however the formatting does not move with them, so any headings below the inserted heading become 12.75 points high, while the cells above those headings become 15 points high.

Sub formatcell()
Dim sht1 As Worksheet
Set sht1 = ActiveSheet

Dim startRow As Integer
Dim endRow As Integer
Dim counter As Integer
Dim DoCell As Range
Dim start As Variant
Dim Finish As Variant

startRow = 16
endRow = FindLastRow(sht1)

'THIS BIT BELOW NEEDS TO BE SPED UP'
start = Timer                 'Start Timer'
counter = startRow
Do While counter < endRow + 1
    Set DoCell = sht1.Cells(counter, 1)
    Debug.Print "DoCell = " & DoCell.Address
    If DoCell.MergeArea.Count > 2 Then
        DoCell.RowHeight = 15
    Else
        DoCell.RowHeight = 12.75
    End If
    counter = counter + 1
Loop
Finish = Timer                            'End timer'
MsgBox = "Time Taken = " & Finish - start

End Sub

This uses the Range.MergeArea.Count method (which identifies the headings as they are 26 cells wide), however I need to move through all of the cells below Row 15 to the end of the sheet and format them all. The problem is that it takes roughly 0.06 seconds per row. This sheet can easily have over 100 rows which need formatting, which is then 6sec I need to wait, which is frustrating.

Is there any other method to do this? Could I give the heading cell a range name (i.e. Heading) when I insert it, and then lookup all cells with that range name?

I'm not sure what's happening on yours but with 10K rows it's doing 1 row per .000090625 seconds (<1 second total) -

Sub test()
Application.ScreenUpdating = False
Dim i As Integer
Dim firstrow As Integer
firstrow = 1
Dim lastrow As Integer
lastrow = 10000
'lastrow = Cells(Rows.Count, 1).End(xlUp).Row
Start = Timer
For i = firstrow To lastrow
    If Cells(i, 1).MergeArea.Count > 1 Then
       Cells(i, 1).RowHeight = 15
    Else: Cells(i, 1).RowHeight = 12.5
    End If
Next
finish = Timer
MsgBox (finish - Start)
Application.ScreenUpdating = True
End Sub

Maybe try a for next loop instead of a do while loop and see if that changes anything?

Nope, I did a do while and it takes the same amount of time, maybe even less?

Do While i < lastrow + 1
    If Cells(i, 1).MergeArea.Count > 1 Then
       Cells(i, 1).RowHeight = 15
    Else: Cells(i, 1).RowHeight = 12.5
    End If
    i = i + 1
Loop

Try Application.ScreenUpdating = False, Application.Calculation = xlCalculationManual, Application.EnableEvents = False and maybe even Application.DisplayAlerts = False - see if those make a difference.

Even making all rows height 90 and running bottom up does 10K rows in less than a second -

For i = lastrow To firstrow Step -1
    If Cells(i, 1).MergeArea.Count > 1 Then
       Cells(i, 1).RowHeight = 30
    Else: Cells(i, 1).RowHeight = 10
    End If
Next

The only other way I could think of not looking for merged cells would be like this -

For i = lastrow To firstrow Step -1
    If Cells(i, 1).Offset(, 1).Column <> 2 Then
       Cells(i, 1).RowHeight = 50
    Else: Cells(i, 1).RowHeight = 20
    End If
Next

But that's no faster.


Lotus Domino API - Parsing through NotesItems I have an application that opens a bunch of .nsf databases (73 for now, and that might increase in the future), reads the documents, and reads the items of these documents. Based on the item name, I get the item value to populate a list of custom objects (the pdbdoc you will see below).

Currently, my code is like this:

NotesDatabase notedb = notesSession.GetDatabase(myrootpath, myDBfilename);
NotesDocumentCollection notesColl = notedb.AllDocuments;
if (notesColl.Count > 0)
{
    NotesDocument doc = notesColl.GetFirstDocument();
    try
    {
        while (doc != null)
        {
            if (doc.Items != null)
            {
                foreach (Domino.NotesItem item in doc.Items)
                {
                    if (item.Values != null)
                    {
                        switch (item.Name)
                        {
                            //PDB DATA
                            case "DocArea": pdbdoc.DocArea = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            case "DocDisc": pdbdoc.DocDisc = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            case "DocObject": pdbdoc.DocObject = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            case "DocOrigin": pdbdoc.DocOrigin = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            case "DocRef": pdbdoc.DocRef = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            case "DocRefClient": pdbdoc.DocRefClient = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            case "DocRevDt": pdbdoc.DocRevDt = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            case "DocRevNo": pdbdoc.DocRevNo = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            case "DocTitle": pdbdoc.DocTitle = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            case "DocUnit": pdbdoc.DocUnit = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            case "INCABINET": pdbdoc.InCabinet = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            //VDB DATA
                            case "VendorDocNm": pdbdoc.DocTitle = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            case "VendorDocRef": pdbdoc.DocRefClient = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            case "VendorDocRevNo": pdbdoc.DocRevNo = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            case "DocUse": pdbdoc.DocObject = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            case "VendorDocRevDt": pdbdoc.DocRevDt = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            case "VendorDocClientRef": pdbdoc.DocRef = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                            case "VendorNm": pdbdoc.VendorName = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
                        }
                    }
                }
            }
            doc = notesColl.GetNextDocument(doc);
        }
    }
    catch (Exception ex)
    {
        //Exception handling
    }
}

I think you can spot my question: is there a cleaner and faster way to parse though NotesItems? I don't know the count of documents I read, but it must be around 700.000, each of them with approx. 140 items, that makes around 100.000.000 items to read. That takes some times..

I tried to use doc.GetItemValue(string itemname), but unfortunately, those databases store various kind of documents, and I am only interested in some of them. GetItemValue would require me to first check of the item exists, and then get its value, so basically, that would read twice the items.

Do any of you have another way to parse thought Lotus documents items?

You can rewrite the nested ifs and loops with a nice and short LINQ query that will give you all the items first:

var items
    from doc in notedb.AllDocuments.Where(doc => doc.Items != null)
    from item in doc.Items.Where(item => item.Values != null)
    select item;

Then, you loop over the items with a foreach loop:

foreach (var item in items)
{
    SetDocProperty(pdbdoc, item);
}

and call a new helper method that encapsulates the switch:

void SetDocProperty(SomeType pdbdoc, SomeOtherType item)
{
    switch (item.Name)
    {
        //PDB DATA
        case "DocArea": pdbdoc.DocArea = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        case "DocDisc": pdbdoc.DocDisc = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        case "DocObject": pdbdoc.DocObject = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        case "DocOrigin": pdbdoc.DocOrigin = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        case "DocRef": pdbdoc.DocRef = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        case "DocRefClient": pdbdoc.DocRefClient = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        case "DocRevDt": pdbdoc.DocRevDt = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        case "DocRevNo": pdbdoc.DocRevNo = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        case "DocTitle": pdbdoc.DocTitle = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        case "DocUnit": pdbdoc.DocUnit = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        case "INCABINET": pdbdoc.InCabinet = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        //VDB DATA
        case "VendorDocNm": pdbdoc.DocTitle = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        case "VendorDocRef": pdbdoc.DocRefClient = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        case "VendorDocRevNo": pdbdoc.DocRevNo = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        case "DocUse": pdbdoc.DocObject = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        case "VendorDocRevDt": pdbdoc.DocRevDt = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        case "VendorDocClientRef": pdbdoc.DocRef = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
        case "VendorNm": pdbdoc.VendorName = LotusUtils.LotusUtils.LotusItemToString(item.Values); break;
    }
}

You could probably parallelize it with Parallel.ForEach but the code isn't complete so I cannot make any other suggestions as I don't know how the pdbdoc works and what it is.


Finding the samallest yellow rectangle I have implemented the following methods in 1 and 2 and need feedback to improve my implementation. Any different approach and ideas are welcome.

  A static method, minRectangle, accepts an array of rectangles (objects of type Rectangle) and returns the rectangle with the smallest area. Create this method.
  A static method, selectRectangles, accepts an array of rectangles (objects of type Rectangle) and a color (an object of type String), and returns those rectangles (as an array) that are of the given color. Create this method.
  Create an array of rectangles (objects of type Rectangle). Then use the methods selectRectangles and minRectangle to determine the smallest of the rectangles in the array that are yellow.

public class Rectangle {

// the lengths of the rectangle’s sides
private double length;
private double width;

// the rectangle’s color
private String color;

public Rectangle(double length, double width, String color) {
    this.length = length;
    this.width = width;
    this.color = color;
}

public String getColor() {
    return color;
}

public double area() {
    return length * width;

}

public String toString() {
    return "<" + this.length + ", " + this.width + "," + this.color + ">";

}

public static Rectangle minRectangle(Rectangle[] rect) {

    Rectangle min = rect[0];
    for (int i = 0; i < rect.length; i++) {
        if (rect[i].area() < min.area()) {
            min = rect[i];
        }
    }
    return min;

}

public static Rectangle[] selectRectangles(Rectangle[] rect, String color) {
    int countRect = 0;
    for (int i = 0; i < rect.length; i++) {
        if (rect[i].getColor().equals(color))
            countRect++;

    }

    Rectangle[] Rect = new Rectangle[countRect];
    int companionVar = 0;
    for (int j = 0; j < rect.length; j++) {
        if(rect[j].getColor().equals(color))
        Rect[companionVar++] = rect[j];

    }
    return Rect;
}

public static void main(String[] args) {
    Rectangle[] rect = {new Rectangle(2, 3, "yellow"),
            new Rectangle(3, 5, "red"),
            new Rectangle(1, 1, "yellow"),
            new Rectangle(2, 2, "red"),
            new Rectangle(3, 4, "red"),
            new Rectangle(2, 8, "red"),
            new Rectangle(5, 2, "yellow")};

    System.out.println("Smallest yellow Rectangle:" + minRectangle(rect));
    Rectangle[] rectangles = selectRectangles(rect, "yellow");
    System.out.println(Arrays.toString(rectangles));
    }

}

The minRectangle is doing what it should but could be improved by storing the value of area() in a variable. Assume you have 1.000.000 rectangles to check then the calculation would happen 2.000.000 times.

public static Rectangle minRectangle(Rectangle[] rect) {

    Rectangle min = rect[0];
    double minArea = min.area();

    for (int i = 0; i < rect.length; i++) {
        double currentArea = rect[i].area();
        if (currentArea < minArea) {
            min = rect[i];
            minArea = currentArea;
        }
    }
    return min;
}

The selectRectangles() method is doing too much work as well. By using an List<Rectangle> to add the rectangles with the desired color and returning the list as array will speed this up.

Please don't omit braces {} although they might be optional. Omitting braces can lead to hidden and therfor hard to track bugs.

public static Rectangle[] selectRectangles(Rectangle[] rect, String color) {
    List<Rectangle> rectangles = new ArrayList<>();
    int countRect = 0;
    for (int i = 0; i < rect.length; i++) {
        if (rect[i].getColor().equals(color)) {
            rectangles.add(rect[i]);
        }
    }

    Rectangle[] results = new Rectangle[rectangles.size()];

    return rectangles.toArray(results);
}

The last task in question isn't solved correctly. You should first get the yellow rectangles in the array and then detect the smallest of these.


Is this HTML structure and CSS DRY enough in this country selection menu? I have built this conceptual design of a localization menu on this CodePen

I am wondering if I should have structured my
s better or if this should be wrapped in a
 instead since
 could be reduced in the CSS with .international li.

HTML

      North America

      Europe

      Korea

CSS

.international {
  background-color: rgb(36, 30, 30);
  color: #fff;
  letter-spacing: 1px;
  padding: 12px 0;
  width: 100%;
}

.locale {
  display: inline-block;
  margin-left: 6px;
  vertical-align: middle;
}
.locale a { color: #fff; text-decoration: none; }

.americas, .europe, .korea {
  background-image: url('https://s3-us-west-2.amazonaws.com/s.cdpn.io/101702/flags-small.png');
  background-repeat: no-repeat;
  margin-top: 3px;
}
.americas {
  background-position: 0px 0px;
  height: 11px;
  width: 48px;
}

.europe {
  background-position: -48px 0px;
  height: 11px;
  width: 48px;
}
.korea {
  background-position: -96px 0px;
  height: 11px;
  width: 16px;
}

.intlButton {
  float: right;
  font-size: 24px;
  margin-right: 6px;
}

.intlButton a { color: #444; }
.intlButton i {
  background-color: rgba(225, 225, 225, .7);
  border-radius: 3px;
  padding: 4px;
}

This should be an ul. Even without CSS you will have a navigational structure. How about giving your nav a better suited name like .nav-localization?

Also I guess the div's inside your links will act like as sub navigations? These would be ul's as well then.

I've added comments to get rid of the whitespace between the list-items, since you're using display: inline-block;. This looks quite dirty, but it's the best solution for the whitespace issue and inline-block.

Here is the markup, I'd go for:

            North America

            Europe

            Korea

How you would select:

.nav-localization {}

.nav-localization > li {}

.nav-localization > li > a {}

.nav-localization .sub-nav {}


Outputting flavor text depending on inputted strings I have this program that takes two strings from the user, stores them in mixArray, then "mixes" them with mixItUp() and outputs a bit of flavor text, depending on the combination. Duplicate combinations (red-blue, blue-red) also give the same outcome.

I've gotten to a nested switch statement that's miles shorter than my previous tries, but I'm trying figure out if there's a more optimal, or readable method to do this, since I intend to scale it up.

mixItUp() {
  this.mixArray.sort();            // Sorts the array
  switch (this.mixArray[0]) {      // Goes through the first object
    case "blue":
      switch (this.mixArray[1]) {  // Goes through the second if the first is "blue"
        case "green":
          console.log("bf");
          break;
        case "purple":
          console.log("bg");
          break;
        case "yellow":
          console.log("bp");
          break;
        default:
          console.log("??");
          break;
      }
      break;
    case "green":     // If [0] is "green"...
      switch (this.mixArray[1]) {
        case "purple":
          console.log("fg");
          break;
        case "yellow":
          console.log("fp");
          break;
        default:
          console.log("??");
          break;
      }
      break;
    case "purple":     // If [0] is "purple"...
      switch (this.mixArray[1]) {
        case "yellow":
          console.log("gp");
          break;
        default:
          console.log("??");
          break;
      }
      break;
    default:
      break;
  }
}

It seems this could be simplified with a small lookup table and then concatenating strings. Using a table as simple as:

const lookup = {
  blue: 'b',
  green: 'f',
  purple: 'g',
  yellow: 'p'
};

we can then lookup the color values and concatenate them together. Combined with a simple check to make sure the color is valid and the 2 colors aren't the same, and it seems to produce the same result.

Here's the full function:

mixItUp: function() {
  this.mixArray.sort();
  const lookup = {
    blue: 'b',
    green: 'f',
    purple: 'g',
    yellow: 'p'
  };

  let mix;
  const color1 = lookup[this.mixArray[0]];
  const color2 = lookup[this.mixArray[1]];

  if (color1 && color2 && (color1 !== color2)) {
    mix = `${color1}${color2}`;
  } else {
    mix = '??';
  }

  return mix;
}

And here it is in action:

function createMix(color1, color2) {
  return {
    mixArray: [color1, color2],
    mixItUp: function() {
      this.mixArray.sort();
      const lookup = {
        blue: 'b',
        green: 'f',
        purple: 'g',
        yellow: 'p'
      };

      let mix;
      const color1 = lookup[this.mixArray[0]];
      const color2 = lookup[this.mixArray[1]];

      if (color1 && color2 && (color1 !== color2)) {
        mix = `${color1}${color2}`;
      } else {
        mix = '??';
      }

      return mix;
    }
  }
}

// Handle the form
const form = document.getElementById('mixitup');

form.addEventListener('submit', event => {
  event.preventDefault();
  const color1 = document.getElementById('color1').value;
  const color2 = document.getElementById('color2').value;

  const mixedArray = createMix(color1, color2)
  console.log(mixedArray.mixItUp())
})
<form id="mixitup">
  <select id="color1">
    <option value="">Select...</option>
    <option value="blue">Blue</option>
    <option value="green">Green</option>
    <option value="purple">Purple</option>
    <option value="yellow">Yellow</option>
  </select>
  <select id="color2">
    <option value="">Select...</option>
    <option value="blue">Blue</option>
    <option value="green">Green</option>
    <option value="purple">Purple</option>
    <option value="yellow">Yellow</option>
  </select>
  <input type="submit" value="Mix it up!">
</form>


Simple Caesar Cipher Function For no particular reason, I wanted to create a function that would take a string and "encrypt" it via Caesar cipher. This function takes a string and shifts the letters left or right (in the alphabet) depending on the input. Shift right(2) for instance would be -

ABCDEFGHIJKLMNOPQRSTUVWXYZ
CDEFGHIJKLMNOPQRSTUVWXYZAB

The UDF:

Option Explicit

Public Function CaesarCipher(ByVal TextToEncrypt As String, ByVal CaesarShift As Long) As String

    'Positive means shift to right e.g. "A" Shift 1 returns "B"
    Dim IsPositive As Boolean
    IsPositive = True
    If CaesarShift < 0 Then IsPositive = False
    CaesarShift = Abs(CaesarShift)

    Dim OutputText As String
    TextToEncrypt = UCase(TextToEncrypt)

    If CaesarShift > 26 Then
        CaesarShift = CaesarShift Mod 26
    End If

    If IsPositive Then
          OutputText = ShiftRight(TextToEncrypt, CaesarShift)
    Else: OutputText = ShiftLeft(TextToEncrypt, CaesarShift)
    End If

    CaesarCipher = OutputText
End Function

The shifting functions:

Private Function ShiftRight(ByVal ShiftString As String, ByVal ShiftQuantity As Long) As String

    Dim TextLength As Long
    TextLength = Len(ShiftString)

    Dim CipherText As String
    Dim CharacterCode As Long
    Dim AsciiIndex As Long
    Dim AsciiIdentifier() As Long
    ReDim AsciiIdentifier(1 To TextLength)

    For AsciiIndex = 1 To TextLength
        CharacterCode = Asc(Mid(ShiftString, AsciiIndex, 1))
        If CharacterCode + ShiftQuantity > 90 Then
            CharacterCode = CharacterCode - 26 + ShiftQuantity
        ElseIf CharacterCode = 32 Then GoTo Spaces
        Else:  CharacterCode = CharacterCode + ShiftQuantity
        End If
Spaces:
        AsciiIdentifier(AsciiIndex) = CharacterCode
    Next

        For AsciiIndex = 1 To TextLength
            CipherText = CipherText & Chr(AsciiIdentifier(AsciiIndex))
        Next
    ShiftRight = CipherText
End Function

Private Function ShiftLeft(ByVal ShiftString As String, ByVal ShiftQuantity As Long) As String

    Dim TextLength As Long
    TextLength = Len(ShiftString)

    Dim CipherText As String
    Dim CharacterCode As Long
    Dim AsciiIndex As Long
    Dim AsciiIdentifier() As Long
    ReDim AsciiIdentifier(1 To TextLength)

    For AsciiIndex = 1 To TextLength
        CharacterCode = Asc(Mid(ShiftString, AsciiIndex, 1))
        If CharacterCode = 32 Then GoTo Spaces
        If CharacterCode - ShiftQuantity < 65 Then
            CharacterCode = CharacterCode + 26 - ShiftQuantity
        Else: CharacterCode = CharacterCode - ShiftQuantity
        End If
Spaces:
        AsciiIdentifier(AsciiIndex) = CharacterCode
    Next

        For AsciiIndex = 1 To TextLength
            CipherText = CipherText & Chr(AsciiIdentifier(AsciiIndex))
        Next
    ShiftLeft = CipherText
End Function

Just some things that jump out at me:

Standard VBA Naming conventions have camelCase for local variables, and PascalCase only for sub/function names and Module/Global Variables. This allows you to tell at a glance if the variable you're looking at is local to your procedure, or coming from somewhere else.

I would probably use EncryptUsingCaesarCypher as your function name. It's more descriptive and closer to what it actually does.

`Text = CaesarCypher(text)

versus

`Text = EncryptUsingCaesarCypher(text)

Other than that, your naming is pretty solid.

Why separate functions for ShiftLeft and ShiftRight? The code in both is heavily-repeated and could be easily combined into a Shift(ByVal shiftValue as Long) function that handles positive and negative. This also lets you cut out all that messing around with isPositve and Abs(shift)


PreparedStatement for closable recordset and connection Following approach uses more Functional approach (non OOP), each method is passed DataSource as dependency.

I've written this piece and I'm wondering if there's something that can be improved. I wish to add ResultSet to try block but not finding any option. Is there a way?

Also please review the class MySQL.

public static Iterator<StateEntity> findAllPrepared(DataSource dataSource, int page, int size) throws SQLException {
    List<StateEntity> list = new ArrayList<>();
    ResultSet rs = null;
    String sql = "SELECT * FROM states LIMIT ?, ?";
    try (
        Connection connection = dataSource.getConnection();
        PreparedStatement statement = connection.prepareStatement(sql, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY);
    ) {
        statement.setInt(1, page);
        statement.setInt(2, size);
        rs = statement.executeQuery();
        while(rs.next()) {
            list.add(new StateEntity(rs.getString("stateId"), rs.getString("stateName")));
        }
    } finally {
        if(rs != null) rs.close();
    }
    return list.iterator();
}

StateEntity

public class StateEntity {

    private String id;
    private String name;

    public StateEntity(String id, String name) {
        this.id = id;
        this.name = name;
    }

    public String getId() {
        return id;
    }

    public String getName() {
        return name;
    }
}

SQL Connection

public class MySQL {

    public static MysqlDataSource dataSource = new MysqlDataSource();

    private static Map<String, String> parameters = new HashMap<String, String>(){
        {
            put("authReconnect", "true");
            put("useSSL", "false");
            put("allowMultiQueries", "true");
            put("useUnicode", "true");
            put("useJDBCCompliantTimezoneShift", "true");
            put("useLegacyDatetimeCode", "false");
            put("serverTimezone", "UTC");
        }
    };

    private static Map<String, String> credentials = new HashMap<String, String>(){
        {
            put("host", System.getenv("HOST") != null ? System.getenv("HOST") : "localhost");
            put("schema", System.getenv("SCHEMA") != null ? System.getenv("SCHEMA") : "usa");
            put("username", System.getenv("USERNAME") != null ? System.getenv("USERNAME") : "root");
            put("password", System.getenv("PASSWORD") != null ? System.getenv("PASSWORD") : "mypass");
        }
    };

    static {
        String query = parameters.entrySet().stream()
                .map(p -> p.getKey() + "=" + p.getValue())
                .reduce((p1, p2) -> p1 + "&" + p2)
                .orElse("");

        dataSource.setURL("jdbc:mysql://" + credentials.get("host")+":3306/" + credentials.get("schema") + "?" + query);
        dataSource.setUser(credentials.get("username"));
        dataSource.setPassword(credentials.get("password"));
    }
}

Test

@Test
public void testGetAllPrepared() throws SQLException {
    Iterator<StateEntity> states = State.findAllPrepared(MySQL.dataSource, 0, 5);
    long count = 0;
    while (states.hasNext()) {
        count++;
        StateEntity stateEntity = states.next();
        System.out.println(stateEntity.getId());
        assertNotNull(stateEntity);
        assertNotNull(stateEntity.getId());
        assertNotNull(stateEntity.getName());
        assertEquals(2, stateEntity.getId().length());
    }
    assertEquals(5, count);
}

Starting out with your first question: no, there isn't. But you may put the result set into another nested try block. Still not super-elegant, but a bit better in my opinion:

try (
    Connection connection = dataSource.getConnection();
    PreparedStatement statement = connection.prepareStatement(sql, ResultSet.TYPE_SCROLL_INSENSITIVE, ResultSet.CONCUR_READ_ONLY);
) {
    statement.setInt(1, page);
    statement.setInt(2, size);
    try (ResultSet rs = statement.executeQuery()) {
        while(rs.next()) {
            list.add(new StateEntity(rs.getString("stateId"), rs.getString("stateName")));
        }
    }

Regarding the method itself: why do you return an iterator instead of the list itself? This severely limits the capabilities of the return value without any benefit. (E.g. you cannot use size() or stream() which you could do with the list.)

Regarding the MySql class now:

Are you aware that you actually create an anonymous subclass with the map construction using curly braces and an object initializer? I never found this on par with the problem solved here. Fortunately, Java 9 brought us Map.of, so I recommend replacing this construct with

private static Map<String, String> parameters = Map.of(
    "authReconnect", "true",
    ...
);

Regarding the query = ...stream...reduce...orElse: you can achive the same by simply using a joining collector:

String query = parameters.entrySet().stream()
    .map(p -> p.getKey() + "=" + p.getValue())
    .collect(Collectors.joining("&"));

The MySql class in itself is just global variables. This is "somewhat ok" as long as you don't employ advanced dependency injection possibilities like spring or a CDI container, but keep in mind that these globals are generally regarded as bad. Advice: read up on dependency injection, and see what options there are.


Write a function that adds two numbers stored in a linked list and returns the sum Just for fun I did the following exercise from "Cracking the coding interview. 4th edition":

  You have two numbers represented by a linked list, where each node
  contains a single digit. The digits are stored in reverse order,
  such that the 1’s digit is at the head of the list. Write a function
  that adds the two numbers and returns the
  sum as a linked list.

  EXAMPLE Input: (3 -> 1 -> 5), (5 -> 9 -> 2)

  Output: 8 -> 0 -> 8

Although my solution it seems to return valid results it is significantly different from the proposed solution. I implemented my own structures and I haven't used recursion.

I know both solutions are valid, but in case of a real interview: How would the interviewer consider my answer? Which caveats should I consider?

public class ex24
{
    public static void Run()
    {

        var n1 = CreateNumber(5, 1, 3);
        var n2 = CreateNumber(2, 9, 5);

        Figure xF = n1;
        Figure xY = n2;
        int alpha = 0;

        Figure prev = null;
        Figure ini;
        do
        {
            int n1v = n1 != null ? n1.Value : 0;
            int n2v = n2 != null ? n2.Value : 0;

            int sum = n1v + n2v + alpha;

            decimal f = sum / (decimal)10;
            alpha = (int)f;
            sum = (int)((f - alpha) * 10);

            if (prev == null)
            {
                ini = new Figure(sum);
                prev = ini;
            }
            else
            {
                prev.Next = new Figure(sum);
                prev = prev.Next;
            }

            n1 = n1 != null ? n1.Next : null;
            n2 = n2 != null ? n2.Next : null;

        } while (n1 != null || n2 != null || alpha != 0);

      // Result is stored on 'ini'.

    }
    public class Figure
    {
        public Figure Next;
        public int Value;

        public Figure(int value)
        {
            this.Value = value;
        }

        public override string ToString() =>"" + this.Value;

    }

    public static Figure CreateNumber(params int[] values)
    {
        Figure ini = new Figure(values[values.Length - 1]);
        Figure prev = ini;
        for (var ix = values.Length - 2; ix >= 0; ix--)
        {
            prev.Next = new Figure(values[ix]);
            prev = prev.Next;
        }

        return ini;
    }

}

Strictly speaking, the code does not solve the problem. As an interviewer I'd expect a clearly defined

    List add(List l1, List l2)

method.

At the very least, rename Run to add, and do not CreateNumbers in it, but pass them as parameters.
Since it is guaranteed that every node contains a digit, the sum may never exceed 19. This means, among other things, that there is no need for decimal. Everything can be done with primitive types, e.g.

    alpha = sum > 10;
    sum -= alpha * 10;

As a side note, the entity you call alpha is traditionally called carry.
The do {} while() approach seems to create more problems than it solves. An immediate manifestation is duplication of n1 != null tests. Consider

    while (n1 != null && n2 != null) {
        // addition logic here
        n1 = n1.Next;
        n2 = n2.Next;
    }

and promote the carry along the remainder of the longer list in a separate loop.
Consider creating a dummy head for a resulting list. It is a standard trick to avoid a special case of prev == null, e.g.

    Figure dummy = new Figure;
    Figure tail = dummy;

    while (....) {
        ....
        tail.next = new Figure(sum);
        tail = tail.next;
    }
    ....
    return dummy.next;


Converts the JSON object to another structure of JSON I want a JSON object of the following structure to plot a chart

chartGroup = [{name: 'A<->B', to: 254, from: 247},
              {name: 'C<->D', to: 208, from: 211},
              {name: 'A<->C', to: 46, from: 41},
              {name: 'A<->D', to: 10, from: 38},
              {name: 'B<->C', to: 56, from: 55},
              {name: 'B<->D', to: 44, from: 45}
             ];

from the below JSON object which I get from Database

data = [{from: "B", to: "D", count: 44},
        {from: "D", to: "B", count: 45},
        {from: "A", to: "B", count: 254},
        {from: "C", to: "D", count: 208},
        {from: "B", to: "A", count: 247},
        {from: "C", to: "A", count: 41},
        {from: "A", to: "D", count: 10},
        {from: "A", to: "C", count: 46},
        {from: "D", to: "C", count: 211},
        {from: "D", to: "A", count: 38},
        {from: "C", to: "B", count: 55},
        {from: "B", to: "C", count: 56}];

To create chartGroup from data, I'm using the below code :

     var chartPoints = {};
     data.forEach(function(s) {
        if((s.from == "A" && s.to == "B") || (s.from == "B" && s.to == "A")) {
            if(s.to == "B"){
                chartPoints["to"] = s.count;
            }else{
                chartPoints["from"] = s.count;
            }
            chartPoints["name"] = "A<->B";

         }
     });
   chartGroup.push(chartPoints);

   var chartPoints = {};
   data.forEach(function(s) {
       if((s.from == "C" && s.to == "D") || (s.from == "D" && s.to == "C")) {
           if(s.to == "D"){
               chartPoints["to"] = s.count;
           }else{
               chartPoints["from"] = s.count;
           }
           chartPoints["name"] = "C<->D";
       }
   });
   chartGroup.push(chartPoints);

   var chartPoints = {};
   data.forEach(function(s) {
       if((s.from == "A" && s.to == "C") || (s.from == "C" && s.to == "A")) {
           if(s.to == "C") {
               chartPoints["to"] = s.count;
           }else{
               chartPoints["from"] = s.count;
           }
           chartPoints["name"] = "A<->C";
    }
  });
  chartGroup.push(chartPoints);

  var chartPoints = {};
  data.forEach(function(s) {
      if((s.from == "A" && s.to == "D") || (s.from == "D" && s.to == "A")) {
          if(s.to == "D") {
             chartPoints["to"] = s.count;
          }else{
             chartPoints["from"] = s.count;
          }
          chartPoints["name"] = "A<->D";
     }
  });
  chartGroup.push(chartPoints);

  var chartPoints = {};
  data.forEach(function(s) {
      if((s.from == "B" && s.to == "C") || (s.from == "C" && s.to == "B")) {
          if(s.to == "C") {
              chartPoints["to"] = s.count;
          }else{
              chartPoints["from"] = s.count;
          }
          chartPoints["name"] = "B<->C";
     }
   });
   chartGroup.push(chartPoints);

   var chartPoints = {};
   data.forEach(function(s) {
       if((s.from == "B" && s.to == "D") || (s.from == "D" && s.to == "B")) {
       if(s.to == "D"){
           chartPoints["to"] = s.count;
       }else{
           chartPoints["from"] = s.count;
       }
       chartPoints["name"] = "B<->D";
    }
  });
  chartGroup.push(chartPoints);

would like to know if there is a better way to reduce the JSON object. As it can be seen that there is lot of loops and conditions which is not good. Please don't give importance to typo as I have modified the data to a example.

Your code had a lot of loops so I tried to squish it into one reducer for you. I think this will work;

var output = data.reduce(function (out, d) {
   var edges = [d.from, d.to].sort();
   var key = edges.join("<->");
   var obj = out.find(function (o) { return o.name === key; });
   if (!obj) {
     obj = {
       name: key,
       from: 0,
       to: 0
     };
     out.push(obj);
   }
   obj.to += edges[0] === d.from ? d.count : 0;
   obj.from += edges[1] === d.from ? d.count : 0;

   return out;
 }, []);

console.log(output);

Output:

[
  {
    "name": "B<->D",
    "from": 45,
    "to": 44
  },
  {
    "name": "A<->B",
    "from": 247,
    "to": 254
  },
  {
    "name": "C<->D",
    "from": 211,
    "to": 208
  },
  {
    "name": "A<->C",
    "from": 41,
    "to": 46
  },
  {
    "name": "A<->D",
    "from": 38,
    "to": 10
  },
  {
    "name": "B<->C",
    "from": 55,
    "to": 56
  }
]

Here's an example on JsBin

Reducers are super powerful when you need to convert an array into a different array or value.

In this case I'm iterating over the data array, and creating the key (A<=>B) by sorting the from and to values.

Then I check the output to see if we've already used that key, if so we append to that existing object, if not then we create it.


FizzBuzz with user input variables Yes, another FizzBuzz! I know you guys may be tired of these but I think it's a tradition stepping stone for beginners on this site.

Here are some notes on my thinking:

I tried to keep it as flexible as possible where the logic is deducted from the variables as much as possible. So one could potentially make a user input form on a website and let the user select all the values (numbers and words), and the code just does it.
I tried to avoid magic numbers as much as I know how. Not sure if 0 counts as a magic number.
I did not feel the need to add any comments as I feel it is self-explanatory. If you feel otherwise, please let me know.

Here is a link to PhpFiddle for your convenience.

<?php
$counter = 1;
$stopper = 100;
$fizzWord = 'Fizz';
$fizzNumber = 3;
$buzzWord = 'Buzz';
$buzzNumber = 5;

for ($counter = 1; $counter <= $stopper; $counter++) {
    if ((($counter %($fizzNumber * $buzzNumber) == 0))) {
        echo $fizzWord . $buzzWord;
    }
    elseif (($counter % $fizzNumber) == 0) {
        echo $fizzWord;
    }
    elseif (($counter % $buzzNumber) == 0) {
        echo $buzzWord;
    }
    else {
        echo $counter;
    }
echo "<br>\n";
}
?>

If your intention is to let users adjust the values used for the fizz and buzz values (3, and 5), then it is possible that they may choose numbers that break your logic quite badly. For example, if they chose '3' and '3', then we would expect all multiples of 3 to be 'FizzBuzz', but, instead, only multiples of 9 would be 'FizzBuzz'. This is because you need to find the lowest common multiple (LCM) of the fizz and buzz values, not just the product of them.

Either you need to calculate the LCM, or you can do them independently using a double test like:

if ($counter % $fizzNumber == 0 && $counter % $buzzNumber == 0) {
    echo $fizzWord . $buzzWord;
....

Notice how I have also removed the redundant (...) braces you had around the conditions. The % operator has a higher precedence than == so it does not need to be elevated.


Better way to structure and optimise try except statements I am new to python and I am trying to write a function with various actions based on the parameters sent. I am checking if the data has valid format then it publishes to /accepted otherwise to /rejected. If the data is fine then it does the function and then publishes to /success otherwise to /failure
Here is what i am doing now -
def data_handler(event, context):
    try:
        input_topic = context.topic
        input_message = event
        response = f'Invoked on topic {input_topic} with message {input_message}'
        logging.info(response)
        client.publish(topic=input_topic + '/accepted', payload=json.dumps({'state': 'accepted'}))
    except Exception as e:
        logging.error(e)
        client.publish(topic=input_topic + '/rejected', payload=json.dumps({'state': 'rejected'}))

    try:
        print('Perform the function based on input message')
        client.publish(topic=input_topic + '/success', payload=json.dumps({'state': 'Execution succeeded'}))
    except Exception as e:
        logging.error(e)
        client.publish(topic=input_topic + '/failure', payload=json.dumps({'state': 'Execution failed'}))

This works as of now but i think there should be a better way to implement this. Any idea?

Given that there isn't much here to analyse purely based on performance since we don't know what those other function calls do, here are some minor changes:
def data_handler(message, topic):
    try:
        logging.debug(f'Invoked on topic {topic} with message {message}')
        client.publish(topic=topic + '/pass', payload=json.dumps({'state': 'pass'}))
    except Exception as e:
        logging.error(e)
        client.publish(topic=topic + '/fail', payload=json.dumps({'state': 'fail'}))

Both try/catch blocks in your code look identical, and you should have only one endpoint that needs to be communicated with. If you really need both, then there should be a separate function to have this code.
It also seems pretty redundant that you're publishing to a pass and fail endpoint and have a payload that says whether or not it passed or failed. You should know that status based on the endpoint that receives the message.
You can have the payload be more descriptive as well to remove multiple endpoints. Something like {'formatted': true, 'error': ''} is far more informative.
Lastly, you can just pass your one-off variables into this function, like so: data_handler(event, context.topic). If your event really is just a post message and not a posting event object, then that should be clarified somewhere in the code base.


Quicksort implementation in Java I wrote this implementation of quicksort as a bit of practice and review for myself.  I did not base my algorithm on anything more than my own understanding of quicksort and working through a couple of examples myself on paper, so please let me know of any improvements or optimizations that I could make!

import java.util.ArrayList;
import java.util.Collections;
import java.util.concurrent.ThreadLocalRandom;

/**
 * A simple, generic, in-place implementation of quicksort
 * @implNote The pivot used during the partitioning step is selected at random
 */
public class Quicksort {

    /**
     * Sort the given array using quicksort
     * @param arr Array to sort
     * @param <E> Type of the elements contained by arr
     */
    public static <E extends Comparable> void sort(ArrayList<E> arr) {
        sort(0, arr.size() - 1, arr);
    }

    /**
     * Sorts a designated section of the given array using quicksort
     * @param left  Lower-bound index of section (inclusive)
     * @param right Upper-bound index of section (inclusive)
     * @param arr   Array to perform sort within
     * @param <E>   Type of the elements contained by arr
     */
    private static <E extends Comparable> void sort(int left, int right, ArrayList<E> arr) {
        // Exit if section contains only a single element, or is invalid
        if (right - left < 1) {
            return;
        }

        // Select a pivot at random
        int pivot = ThreadLocalRandom.current().nextInt(left, right + 1);

        // Partition this section of the array
        int pivotFinalIndex = partition(left, right, pivot, arr);

        // Sort the two new partitions
        sort(left, pivotFinalIndex - 1, arr);
        sort(pivotFinalIndex + 1, right, arr);
    }

    /**
     * Partition a section of an array around a pivot value
     * @param left  Lower-bound index of section (inclusive)
     * @param right Upper-bound index of section (inclusive)
     * @param pivot Index of the value to partition around
     * @param arr   Array to perform partitioning within
     * @param <E>   Type of the elements contained by arr
     * @return      The final index of the partition value
     */
    private static <E extends Comparable> int partition(int left, int right, int pivot, ArrayList<E> arr) {
        // Move pivot to left-most position (get out of the way)
        Collections.swap(arr, left, pivot);
        pivot = left;

        // Perform partitioning
        int rightPartitionStart = left + 1;
        for (int i = left + 1; i <= right; i++) {
            // If our current value is less than our pivot, move it into the left partition
            if (arr.get(i).compareTo(arr.get(pivot)) < 0) {
                Collections.swap(arr, i, rightPartitionStart);
                rightPartitionStart++;
            }
        }

        // Put pivot back where it belongs (in between partitions)
        Collections.swap(arr, pivot, rightPartitionStart - 1);
        pivot = rightPartitionStart - 1;

        return pivot;
    }
}

1

I would declare <E extends Comparable> void sort(ArrayList<E> arr) as <E extends Comparable<? super E>> void sort(ArrayList<E> arr)

2

void sort(int left, int right, ArrayList<E> arr) {
    ...
}

I suggest you make it public and reorganize the arguments like this:

void sort(ArrayList<E> arr, int fromIndex, int toIndex)

where fromIndex corresponds to left in your version, and toIndex corresponds to right + 1: these are the conventions in JDK's sort algorithms, so that toIndex is an exclusive upper bound.

3

I would make the three-parameter version public since sometimes people want to sort only a subrange of an array (or ArrayList in our case).

4

Declare the constructor as private:

private Quicksort() {}

Hope that helps.