Scanning and indexing files video I use the following method to scan and index video files and pass data to an array. I get video file duration, file name, files size.
Is there any better method, or do I need to improve?
public static void load_Directory_Files(File directory) {
    File[] fileList = directory.listFiles();
    if(fileList != null && fileList.length > 0){
        for (int i=0; i<fileList.length; i++){
            if(fileList[i].isDirectory()){
                load_Directory_Files(fileList[i]);
            } else {
                String name = fileList[i].getName().toLowerCase();
                for (String extension: Constant.videoExtensions){
                    //check the type of file
                    if(name.endsWith(extension)){
                        Constant.allMediaList.add(fileList[i]);
                        String FileName = fileList[i].getName();
                        String size = Function.humanReadableByteCountSI(fileList[i].length());
                        String duration = Function.getDurationFF(fileList[i].getAbsolutePath());
                        String folderName = fileList[i].getParentFile().getName();
                        String path = fileList[i].getAbsolutePath();
                        Log.d(TAG, "load_Directory_Files: "+ duration + "" +size + "" + FileName);
                        Constant.allMediaListWithModel.add(new AllVideoModel(FileName, path, folderName, size, duration));
                        //when we found file
                        break;
                    }
                }
            }
        }
    }
}

public static String getDurationFF(String absolutePathThumb) {
    try{
        FFmpegMediaMetadataRetriever retriever = new FFmpegMediaMetadataRetriever();
        retriever.setDataSource(absolutePathThumb);
        String time = retriever.extractMetadata(FFmpegMediaMetadataRetriever.METADATA_KEY_DURATION);
        long timeInMillisec = Long.parseLong(time);
        return convertMillieToHMmSs(timeInMillisec);
    }catch(Exception e){
        e.printStackTrace();
        return null;
    }
}

@SuppressLint("DefaultLocale")
public static String convertMillieToHMmSs(long millie) {
    long seconds = (millie / 1000);
    long second = seconds % 60;
    long minute = (seconds / 60) % 60;
    long hour = (seconds / (60 * 60)) % 24;
    if (hour > 0) {
        return String.format("%02d:%02d:%02d", hour, minute, second);
    } else {
        return String.format("%02d:%02d" , minute, second);
    }
}

@SuppressLint("DefaultLocale")
public static String humanReadableByteCountSI(long bytes) {
    if (-1000 < bytes && bytes < 1000) {
        return bytes + " B";
    }
    CharacterIterator ci = new StringCharacterIterator("kMGTPE");
    while (bytes <= -999_950 || bytes >= 999_950) {
        bytes /= 1000;
        ci.next();
    }
    return String.format("%.1f %cB", bytes / 1000.0, ci.current());
}

The above method works but it takes time to scan and index. So how can I reduce indexing time?
public static String[] videoExtensions = {".mp4",".ts",".mkv",".mov",
        ".3gp",".mv2",".m4v",".webm",".mpeg1",".mpeg2",".mts",".ogm",
        ".bup", ".dv",".flv",".m1v",".m2ts",".mpeg4",".vlc",".3g2",
        ".avi",".mpeg",".mpg",".wmv",".asf"};

Naming
The method name load_Directory_Files is a bit odd. To follow general Java naming conventions, it would be something like loadDirectoryFiles. This is called camel case.
File filtering
You can use a FileFilter to accept or discard files for the
listFiles call, no need to do it yourself:
import java.io.File;
import java.io.FileFilter;
import java.util.List;
import java.util.Arrays;

public class VideoOrDirectoryFilter implements FileFilter {

    private static final List<String> ALLOWED_EXTENSIONS = Arrays.asList(
        ".mp4",".ts",".mkv",".mov",".3gp",".mv2",".m4v",".webm",
        ".mpeg1",".mpeg2",".mts",".ogm", ".bup", ".dv",".flv",
        ".m1v",".m2ts",".mpeg4",".vlc",".3g2",".avi",".mpeg",
        ".mpg",".wmv",".asf"
    );

    @Override
    public boolean accept(File file) {
        return file.isDirectory()
            || ALLOWED_EXTENSIONS.contains(file.getName().toLowerCase());
    }
}

Null check
In loadDirectoryFiles, you can return early if the listFiles call is null:
File[] files = directory.listFiles(new VideoOrDirectoryFilter());
if (files == null) return;

Looping
Instead of the for loop with an index variable, you could use a foreach loop. That makes for cleaner code, as you directly get the object in the list. Also, you don't need to check the length of the list before looping. If it's empty, the loop will be a no-op.
for (File file : files) {
    // Do your thing with file here
}

Reimplementation
Here's a reimplementation of the loadDirectoryFiles method with the above enhancements included.
public static void loadDirectoryFiles(File directory) {
    File[] files = directory.listFiles(new VideoOrDirectoryFilter());

    if (files == null) return;

    for (File file : files) {
        if (file.isDirectory()){
            loadDirectoryFiles(file);
        } else {
            Constant.allMediaList.add(file);
            String fileName = file.getName();
            String size = Function.humanReadableByteCountSI(file.length());
            String duration = Function.getDurationFF(file.getAbsolutePath());
            String folderName = file.getParentFile().getName();
            String path = file.getAbsolutePath();
            Log.d(TAG, "load_Directory_Files: "+ duration + "" +size + "" + fileName);
            Constant.allMediaListWithModel.add(new AllVideoModel(fileName, path, folderName, size, duration));
        }
    }
}


NSAutoreleasePool with Json Data Just need to know if I'm doing something wrong in this code. my app seem work fast now with this code. I just want to if i really understant that

   - (void)receive
{

    NSString *post2 = [NSString stringWithFormat:@"expediteur=%@&destinataire=%@",
                       [[expediteurLbl text] stringByAddingPercentEscapesUsingEncoding:NSUTF8StringEncoding],
                       [[destinataireLbl text] stringByAddingPercentEscapesUsingEncoding:NSUTF8StringEncoding]];

    NSData *dataToSend2 = [NSData dataWithBytes:[post2 UTF8String] length:[post2 length]  ];

    request2 = [[[NSMutableURLRequest alloc] init] autorelease];
    [request2 setURL:[NSURL URLWithString:@"http:/****************.php"]];
    [request2 setHTTPMethod:@"POST"];
    [request2 setHTTPBody:dataToSend2];
    [request2 setValue:@"application/x-www-form-urlencoded" forHTTPHeaderField:@"content-type"];

    [NSThread detachNewThreadSelector:@selector(displayView) toTarget:self withObject:nil];

    }

    -(void)displayView

    {
        NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
        NSURLResponse *response2;
        NSError *error2;
        NSData *data2 = [NSURLConnection sendSynchronousRequest:request2 returningResponse:&response2 error:&error2];
        reponseServeur2= [[NSMutableString alloc] initWithData:data2 encoding: NSASCIIStringEncoding];

        responseString2 = [[NSString alloc] initWithData:data2 encoding:NSUTF8StringEncoding];

        [[reponseServeur2 stringByReplacingOccurrencesOfString:@"\n"  withString:@""] mutableCopy];

        self.messageArray = [responseString2 JSONValue];
        [messTableView reloadData];

        [pool release];

    }

and

- (void)viewDidLoad {
    [super viewDidLoad];

    [self performSelectorInBackground:@selector(receive) withObject:nil];
}

I gather that request2 is an instance variable?  It's risky at best to set it with an autoreleased value and then expect it to be valid later in your displayView method.

But it probably works because you execute receive in a background task, and background tasks have no default autorelease pool.  If it weren't for that request2 would likely go "poof" before it got to displayView.

So all of the autoreleased data items in receive -- post2, dataToSend2, request2, several temporary strings, and at least one temporary URL -- are leaking.


How to make this random number game better? I'm a newbie trying to teach myself Python. Could somebody tell me if there is a better way to write this?  I am also curious as to whether or not there is anything I should not do.  This code does work, but some indentation may have been lost when I pasted it.

import random

def game():
    yes = ['y', 'Y', 'Yes', 'yes']
    number = random.randint(1, 20)
    numAttempts = 0
    print('')
    name = input('What is your name? ')

    print('')
    print('Nice to meet you', name, '! I am going to guess a random number! You will have 3 tries to guess it. '
                                'Good luck!')

    while numAttempts < 3:
        print('')
        guess = int(input('Guess the number I have chosen between 1 and 20. '))
        numAttempts += 1
        if guess == number:
            print('')
            print('You guessed correctly! The number was', number, "!", 'Great Job', name, '!')
            break
        elif guess > number:
            print('')
            print('The number is lower than', guess, "!")
        elif guess < number:
            print('')
            print('The number is Higher than', guess, "!")
    else:
        print('')
        print('Sorry, the number I chose was', number, 'but you have used all 3 tries', name, '!')

    print('')
    gameover = input("Would you like to play again? Yes or No? ")
    while gameover in yes:
        game()
    else:
        print('')
        print('You should have chose yes. You computer will now explode!')

game()

A few observations:

In place of the while loop with you managing the numAttempts index,  why not use a for loop with the in keyword? i.e. for numAttempts in range(3):
Note that with the above, you can still use your else block (for supports the else clause! :)  )
Multiple games keep asking for your name. Unless your code has Alzheimer's, consider avoiding that :)
Related to the above : Structurally, you've chosen to recurse to play another game. Why not keep your game atomic and keep the whole offer another game choice/logic external to your function?
Your second elif does not make sense. Why not just else (guess can only be < number now!)


Project Euler Problem #3 - largest prime factor I have began doing the problems found on Project Euler and have just solved problem 3. However, I feel as if my code is very long and too brute-force. I was hoping somebody could give it a look and give me some guidelines for this problem and for future programming in general.  This would help me with cleaning it up or even just revealing better ways of solving this.

Problem 3 states:

  The prime factors of 13195 are 5, 7, 13 and 29.

  What is the largest prime factor of the number 600851475143?

int primeFactor = 0;
int pfCounter = 0;
int[] pf = new int [1229];
int[] numbers = new int[10000];
long number = 600851475143L;
int p = 0;

//initialize the array numbers to include all numbers
//from 1 to 1000 for determining prime numbers
for(int i = 0; i < numbers.length; i++)
{
    numbers[i] = i+1;
}

//determining which numbers are prime numbers
for(int i = 0; i <numbers.length; i++)
{
    //divide each number by every number below it
    //to see if the pfCounter = 1 which would mean
    //it is only divisible by itself and 1
    for(int j = i; j > 0; j--)
    {
        if(numbers[i] % j == 0)
        {
            pfCounter++;
        }
    }

    //if the number only has 1 in it's pfCounter
    //insert the number from the numbers array into the
    //pf array and restart the counter to 0 else restart
    //the counter to 0 and try again
    if(pfCounter == 1)
    {
        pf[p] = numbers[i];
        pfCounter = 0;
        p++;
    }
    else
    {
        pfCounter = 0;
    }

}

/*
for(p = 0; p < pf.length; p++)
{
    System.out.println(pf[p]);
}*/

for(int i = 0; i < pf.length; i++)
{
    if(number % pf[i] == 0)
    {
        primeFactor = pf[i];
    }
}

System.out.println(primeFactor);

If a number x (with x > 2) is not a prime number, there exists a prime p so that x is divisible by p and p < x.

This means to determine if a number is a prime number, you only have to test it against smaller primes (which you already know). We need one prime (2) to start this process, so we will begin to search for primes from 3 onward.

This technique is known as memoization, which you will find quite useful for many PE problems.

This would lead to the following code (incorporating the points of GameDroids):

int[] primes = new int[somethingSufficientlyLarge];
primes[0] = 2;
primeCounter = 1; // a "pointer" to the next prime

for (int n = 3; n < 10000; n += 2) { // only odd numbers can be primes (except 2)
  bool isAPrime = true;
  for (int i = 0; i < primeCounter; i++) {
    if (n % primes[primeCounter] == 0) {
      isAPrime = false;
      break;
    }
  }
  if (isAPrime) {
    primes[primeCounter] = n;
    primeCounter++
  }
}

Note that using a (Linked) List will make hardcoding the length of the prime array unneccessary. However, there may or may not be performance implications.

But you do not know that the largest prime factor is in fact smaller than 10000. It could be ⌊600851475143/2⌋ for all we know. This would be the proper bound of your search space.

Maybe we can reduce that bound: Every time we find a prime in our sieve loop, we test if number is divisible by that prime. We then do so until the number isn't divisible by that factor. If the result is 1, we can break the loop, as the largest prime factor was found.
The loop runs from 2 to bound, where bound initially is floor(number/2), but once the first factor is found, the bound is the number divided by every factor. If no factor was found, then the number itself is the largest factor.

Example 1:

Factorization of the number 15. Bound is floor(15/2) = 7.
First prime is 2. Not a factor.
Next candidate 3 is prime, is a factor. New bound is 5.
Next candidate 5 is prime, is a factor. Bound drops to 1 → break.
Largest prime factor is 5.

Example 2:

Factorization of the number 13. Bound is floor(13/2) = 6.
First prime is 2. Not a factor.
Next candidate 3 is prime, not a factor.
Next candidate 5 is prime, not a factor.
Loop exits without factor found. 13 itself must be a prime, and its largest prime factor.

General hints:

In your original code you tested for factors in the range (0, n] where n is the number you wanted to factorize. But as no factor can be larger than n/2, you could have reduced the range to (0, ⌊n/2⌋]. This would have reduced substantial overhead. Try to find such cases that can never occur (factors in the range [⌈n/2⌉, n]) and optimize.
In your fist factorization loop you started with the largest factor first. This is bad, as ½ of numbers are divisible by 2, ⅓ by 3, ⅕ by 5 and so on. I.e. you can weed out many non-primes by testing a very small set of numbers. Try to find common cases, and test for them befoe testing for uncommon cases. Also, try to “fail early” (which is wisdom not only applicable to programming).

However, when factorizing your large number, you should have started with the largest possible prime first, count downwards, and stop once you found a factor, instead of looping through all primes


Smoke the SmokeDetector In Tavern on the Meta, our spam detector is a bot named SmokeDetector.

While you can ignore the bot, you can't ignore specific replies to Smokey without ignoring users, so I wrote a UserScript to hide SmokeDetector messages and replies/calls to it by other users.

Click Here to install. It's also available on GitHub and StackApps.

I used the startsWith and includes polyfills, because I was too lazy to use indexOf.

I tried to get as much duplicate logic out of it as possible, hence the innerText function, but it just feels icky at points. I'm not particularly sure how to improve it beyond extracting logic out of the main function a bit more.

// ==UserScript==
// @name SmokeTheSmokeDetector
// @description No more smokey spam
// @version 0.0.1
// @match *://chat.stackexchange.com/rooms/*
// @match *://chat.stackoverflow.com/rooms/*
// @match *://chat.meta.stackexchange.com/rooms/*
// @author The-Quill
// @downloadURL  https://github.com/The-Quill/SmokeTheSmokeDetector/raw/master/SmokeTheSmokeDetector.user.js
// @updateURL https://github.com/The-Quill/SmokeTheSmokeDetector/raw/master/SmokeTheSmokeDetector.user.js
// @run-at document-end
// ==/UserScript==

if (!String.prototype.startsWith) {
    String.prototype.startsWith = function(searchString, position) {
        position = position || 0;
        return this.substr(position || 0, searchString.length) === searchString;
    };
}
if (!String.prototype.includes) {
    String.prototype.includes = function(search, start) {
        'use strict';
        if (typeof start !== 'number') {
            start = 0;
        }
        if (start + search.length > this.length) {
            return false;
        }
        return this.indexOf(search, start) !== -1;
    };
}
var SmokeyIdsBasedOnSite = {
    "chat.meta.stackexchange.com": 266345,
    "chat.stackexchange.com": 120914,
    "chat.stackoverflow.com": 3735529
};
var SmokeyUserId = SmokeyIdsBasedOnSite[document.location.host];
function innerText(element, selector){
    if (!selector) return "";
    var foundElement = element.querySelector(selector);
    if (!foundElement || !foundElement.innerText) return "";
    return foundElement.innerText;
}
function hideSmokeyMessage(message){
    var messageBlock = message.parentNode.parentNode;
    if (messageBlock.classList.contains('user-' + SmokeyUserId)){
        var text = innerText(message, '.content');
        if (!text.includes('fault.') && !text.includes('privileged user')){
            var parentNode = message.parentNode;
            parentNode.removeChild(message);
            if (parentNode.children.length === 0){
                messageBlock.style.display = "none";
            }
        }
        return;
    }
    var messageText = innerText(message, '.content');
    var isSmokeyResponse = innerText(message, '.mention') === '@SmokeDetector';
    if (messageText.startsWith('sd') || (messageText.startsWith('!!/') && messageText !== '!!/blame' && messageText !== '!!/amiprivileged') || isSmokeyResponse){
        var parentNode = message.parentNode;
        parentNode.removeChild(message);
        if (parentNode.children.length === 0){
            messageBlock.style.display = "none";
        }
        return;
    }
}
function nodeInsertionListener(){
    var messages = document.querySelectorAll(".monologue .message");
    Array.prototype.slice.apply(messages).forEach(hideSmokeyMessage);
}
window.addEventListener("DOMNodeInserted", nodeInsertionListener);

I'm sorry, but my review is going to be short; your code looks pretty good.

Refactor node deletion

You did a very good job of refactoring your code for the most part. The only repetition that I can find is here:

var parentNode = message.parentNode;
parentNode.removeChild(message);
if (parentNode.children.length === 0){
    messageBlock.style.display = "none";
}

Perhaps it would be best to split this off into a deleteMessage function to keep things simple.

Refactoring message content checking

This conditional is fairly lengthy:

if (messageText.startsWith('sd') || (messageText.startsWith('!!/') && messageText !== '!!/blame' && messageText !== '!!/amiprivileged') || isSmokeyResponse)

Perhaps it would be cleaner to split this off into a separate function called isSDCommand.

Variable confusion

In the first part of hideSmokeyMessage, you have this:

var text = innerText(message, '.content');

In the second part, after the conditional, you have this:

var messageText = innerText(message, '.content');

Two variables, same exact content but with different names and in different scopes. That's a little confusing.

You should define the text variable above the first conditional so both parts of the function can use it, since they both need to use it anyway.

Simplifying argument checking

This is your innerText function:

function innerText(element, selector){
    if (!selector) return "";
    var foundElement = element.querySelector(selector);
    if (!foundElement || !foundElement.innerText) return "";
    return foundElement.innerText;
}

First off, if you are going to check for the existence of selector, why not also check for the existence of element? I personally think you could just drop the checking all together because

This isn't some standard function that you are reinventing
You always provide the second parameter

Another simplification you can make is removing the second part of the remaining conditional (the part after the ||). If there is no text, it's going to be "" anyway.

Now your function looks like this:

function innerText(element, selector){
    var foundElement = element.querySelector(selector);
    if (!foundElement) return "";
    return foundElement.innerText;
}

This can be even further simplified with a ternary operator now:

return foundElement ? foundElement.innerText : "";

Nitpick

You can merge the SmokeyIdsBasedOnSite map and the SmokeyUserId together to make it a bit cleaner in that section:

var SmokeyUserId = ({
    "chat.meta.stackexchange.com": 266345,
    "chat.stackexchange.com": 120914,
    "chat.stackoverflow.com": 3735529
})[document.location.host];


Check Abecedarian word An 'abecedarian' is a word whose letters appear in alphabetical order.
  Write a program which will determine whether the given word is
  abecedarian or not and print 1 if abecedarian, 0 if not.

Input : user enter a string or word.
Output: print `1` if word is Abecedarian else `0`.

Test Cases :

abc    1
beee   1
cuanf  0

#include<stdio.h>
#include<string.h>

void main()
 {

   char str[20];

printf("\n Enter a string");

scanf("%s",str);

int len = strlen(str);

int i,flag = 1;

for(i=0;i < len - 1;i++)
 {

if(str[i+1] >= str[i])

    continue;

else
{
       flag = 0 ;
   break;
}
}

if(flag == 0)

printf("\n String is not Abecedarian ");

else

printf("\n Is abecedarian");

}

Formatting

The obvious wrong thing with your code is the indentation (I don't know if it is because of the copy on CodeReview.SE).

Also, the lines breaks seem to be used randomly. They should be used to divide the different logical parts of your function/program.

You could use more spaces to make your code easier to go through.

Finally (but this is a tiny detail), you have loads of trailing whitespace making things a bit more painful to edit. You can easily remove them automatically.

Activate warnings in your compiler

It is usually a good idea to enable all warnings in your compiler and to take them into account. In your case, gcc complains about the return value for main.

Once this is fixed, your code looks like :

#include <stdio.h>
#include <string.h>

int main()
{
        char str[20];
        printf("\n Enter a string");
        scanf("%s",str);
        int len = strlen(str);
        int i,flag = 1;
        for(i=0; i < len - 1; i++)
        {
                if (str[i+1] >= str[i])
                        continue;
                else
                {
                        flag = 0 ;
                        break;
                }
        }

        if(flag == 0)
                printf("\n String is not Abecedarian ");
        else
                printf("\n Is abecedarian");
        return 0;
}

Logic

Your code seems OK but there is a way to improve how the logic is written. For instance, you could get rid of the continue by rewriting the condition. The inside of the loop becomes :

            if (str[i+1] < str[i])
            {
                    flag = 0 ;
                    break;
            }

In the same kind of spirit, I'd inverse the conditions for the printing as I find it easier to read something like :

    if(abcdarian)
            printf("\n Is abecedarian");
    else
            printf("\n String is not Abecedarian ");

More style

It is usually a good idea to try to move your variables in the smallest possible scope. In your case, using c99, you can move i into the for loop : for(int i=0; i < len - 1; i++).

In the same kind of spirit, it could be a good idea to create a function to handle the logic currently in the loop. Then you'd be able to just call if (isAbecedarian(str)).

About scanf

You'll find online a few problems with scanf and details about what you should be doing like using fgets.


Simple Book List App I've built a simple Book list app. It allows users to enter the name, author and isbn of a book into a form. Then when this is submitted successfully, the details get added to a new row in the table.

I've tried to make the code as optimal as possible. Any feedback would be greatly appreciated.

const bookList = document.getElementById('book-list');

class Book {
  constructor(title, author, isbn){
    this.title = title;
    this.author = author;
    this.isbn = isbn;
  }

  addBook(){
    const bookList = document.getElementById('book-list');
    const bookItem = document.createElement('tr');

    bookItem.innerHTML =`
      ${this.title}
      ${this.author}
      ${this.isbn}
      <button type="submit" class="delete">X</button>	`;

    return bookList.appendChild(bookItem);
  }

  static deleteBook(target){
    target.originalTarget.parentElement.parentElement.remove();
  }
}

// Event Listener: Form Submit
form.addEventListener('submit', function(e){
  const bookFields = getFields();

  if(!validateForm(bookFields)) {
    showNotification('Error', 'error');
    e.preventDefault();
    return false;
  }

  const book = new Book(
    bookFields.title,
    bookFields.author,
    bookFields.isbn
  );

  book.addBook(book);

  showNotification('Success', 'success');
  this.reset(); // Reset Form

  e.preventDefault();
});

// Event Listener: Delete Book
bookList.addEventListener('click', function(e){
  Book.deleteBook(e);
  showNotification('Deleted Item', 'success');
});

function getFields(){
  return fields = {
    'title': document.getElementById('title').value,
    'author': document.getElementById('author').value,
    'isbn': document.getElementById('isbn').value
  };
}

function validateForm(fields){
  for(let [key, value] of Object.entries(fields)){
    if(value === '') return false;
  }
  return true;
}

// Notification
function showNotification(message, type){
  const container = document.querySelector('.container');
  const alert = document.createElement('div');

  alert.classList = `${type}`;
  alert.appendChild(document.createTextNode(message));

  container.insertBefore(alert, form);

  setTimeout(function(){
    alert.remove();
  }, 1500);
}```

Some possible improvements:

You select the bookList multiple times. You only need to select it once, in the outer scope, and let lexical scoping do the rest - you can remove the line that re-declares it without issues.

Inserting characters from user input directly into a string of HTML markup can occasionally result in unexpected elements appearing when the input contains < brackets. (This can also allow for arbitrary code execution)

const bookItem = document.querySelector('tr');
const title = 'Title <Subtitle in brackets>';
const author = '<img src onerror="alert(\'evil\');"';
bookItem.innerHTML =`
  ${title}
  ${author}
`;

Either escape < brackets first, or insert the s without content, then select them afterwards and assign to their textContent (which won't have any escaping issues).

${this.title.replace(/</g, '&lt;')}
${this.author.replace(/</g, '&lt;')}
${this.isbn.replace(/</g, '&lt;')}
<button type="submit" class="delete">X</button>	`;

The return value from addBook is not used, so there's no need for it to return a value. Unless you're intending consumers of Book to use the created
, you can remove the return from the end of the function.

Your deleteBook function is odd - whenever anything in the bookList is clicked, it will remove the grandparent element. This does not seem desirable. You probably want to remove a
 only when the X button is clicked. Add an event listener to the button instead:

addBook(){
  const tr = document.createElement('tr');
  tr.innerHTML =`
  ${this.title.replace(/</g, '&lt;')}
  ${this.author.replace(/</g, '&lt;')}
  ${this.isbn.replace(/</g, '&lt;')}
  <button type="submit" class="delete">X</button>	`;
  const button = tr.querySelector('button');
  button.addEventListener('click', () => {
    tr.remove();
    showNotification('Deleted Item', 'success');
  });
  bookList.appendChild(tr);
}

You could also keep using your listener on the whole bookList, but you'd have to make sure that the clicked element is the X button before removing elements - check if the target .matches('.delete'). (but even then, it's strange for that functionality to be outside of the Book class - it would make more sense for it to be encapsulated)

If you do use the delegation method, use event.target, not event.originalTarget (which is non-standard and only works on Firefox - on any other browser, it will throw an error). (also, best to name the variables appropriately - you're passing the event to deleteBook, not the event.target, so calling the parameter target is confusing)

You can make the submit listener a bit less repetitive by calling e.preventDefault on the first line of the function, that way you don't have to write it twice later, nor do you have to worry about accidentally not calling it in case you add more logical paths to the function. (And if you call preventDefault(), there's no need to return false, nothing needs to be returned)

In getFields, you're implicitly creating a global variable fields. This doesn't do anything useful, and will throw an error in strict mode (which you should be using). Just remove the fields = part:

function getFields(){
  return {
    'title': document.getElementById('title').value,
    'author': document.getElementById('author').value,
    'isbn': document.getElementById('isbn').value
  };
}

Doesn't matter much here, but if you had more fields, to be less repetitive, you could replace that with:

function getFields() {
  return Object.fromEntries(
    ['title', 'author', 'isbn'].map(
      id => [id, document.getElementById(id).value]
    )
  );
}

It's good to prefer const whenever possible - using let is a warning to future readers of the script (including yourself) that you may be reassigning the variable later. So, you could replace

for(let [key, value] of Object.entries(fields)){

with

for(const [key, value] of Object.entries(fields)){

Or, even better, since you don't care about the keys at all, only that none of the values are empty:

function validateForm(fields){
  return Object.values(fields).every(Boolean);
}

When alert is used as an identifier in Javascript, it usually refers to window.alert. Calling a different variable alert can be confusing. Consider changing

const alert = document.createElement('div');

to something like alertDiv.

Template literals look nicer than concatenation with +, but if there's no concatenation going on, and there aren't any ' or " characters causing escaping issues, there's no point in a template literal. Replace

alertDiv.classList = `${type}`;

with

alertDiv.classList = type;

But the classList is generally understood to be a read-only DOMTokenList. While you can change the class of an element by assigning to the classList property, it looks quite odd. If you want to assign a class, either use classList.add, or (if you don't care about overwriting the existing class) assign to the className property:

alertDiv.className = type;

Using document.createTextNode is often unnecessarily verbose. You can just assign to the textContent of the element instead. Replace

alertDiv.appendChild(document.createTextNode(message));

with

alertDiv.textContent = message;


Asynchronously fetching Bitcoin orders and quotes I have the following code to get data using async/await

import fetch from 'node-fetch';

let bid = 0;
let ask = 0;
let lastBuy = 0;
let buyOrders = [];
let sellOrders = [];
const currency = process.argv[3].toUpperCase();

const getOrderBook = async () => {
  const orderBook = await fetch(`https://bittrex.com/api/v1.1/public/getorderbook?market=BTC-${currency}&type=both`)
    .then(response => response.json())
    .then(response => response.result)
    .then(result => ({ buys: result.buy.slice(0, 10), sells: result.sell.slice(0, 10) }));

  return orderBook;
};

const getTicker = async () => {
  const ticker = await fetch(`https://bittrex.com/api/v1.1/public/getticker?market=BTC-${currency}`)
    .then(response => response.json())
    .then(response => response.result);

  return ticker;
};

const getCurrencyData = () => {
  getOrderBook().then(orderBook => ([buyOrders, sellOrders] =  [orderBook.buys, orderBook.sells] )); //eslint-disable-line
  getTicker().then(ticker => ([bid, ask, lastBuy] = [ticker.Bid, ticker.Ask, ticker.Last])); //eslint-disable-line
};

setInterval(() => {
  getCurrencyData();
}, 5000);

This is the first time I use async/await, so I have the following doubts:

First, on my getOrderBook and getTicker function I can't find a way to return the values without first assigning to a variable.

Also, it looks very ugly in the getCurrencyData function, because after I called the functions I have to make a ".then" to assign the data to my variables.

One of the main benefits of using await is to keep your code flat and to avoid .thens - you're not doing that in getOrderBook. nor in getTicker. If you want to use await, try awaiting each asynchronous operation.

Accessing a property of an object is not an asynchronous operation, nor is creating a new object: those lines should not be in .thens.

Because both fetch() and .json() access the .result property, you can keep from repeating yourself by putting that into another function:

const fetchBittrexResult = async (method) => {
  const params = 'market=BTC-' + currency + (method === 'getorderbook' ? '&type=both' : '');
  const url = `https://bittrex.com/api/v1.1/public/${method}?${params}`;
  const response = await fetch(url);
  const responseObj = await response.json();
  return responseObj.result;
};

Then consume it:

const getOrderBook = async () => {
  const { buy, sell } = await fetchBittrexResult('getorderbook');
  return {
    buys: buy.slice(0, 10),
    sells: sell.slice(0, 10),
  };
};

const getTicker = () => fetchBittrexResult('getticker');

(no need for getTicker to be async anymore)

For getCurrencyData, I'm doubtful that continually re-assigning top-level variables is a good way to do this, but without seeing the consumers of the fetched data, it's hard to say what the better solution is. Argument destructuring will help reduce the syntax noise there, and don't feel like you have to squash everything onto one long line:

const getCurrencyData = () => {
  getOrderBook()
    .then(({ buys, sells }) => ([buyOrders, sellOrders] =  [buys, sells] ));
  getTicker()
    .then(({ Bid, Ask, Last }) => ([bid, ask, lastBuy] = [Bid, Ask, Last]));
};

It would probably be better to assign to an object rather than to multiple standalone variables, though.


Quick sort algorithm Is this algorithm good or can I improve it?

 static void QuickSort(int[] a)
    {
        QuickSort(a, 0, a.Length - 1);
    }

    static void QuickSort(int[] a, int start, int end)
    {
        if (start >= end)
        {
            return;
        }

        int num = a[start];

        int i = start, j = end;

        while (i < j)
        {
            while (i < j && a[j] > num)
            {
                j--;
            }

            a[i] = a[j];

            while (i < j && a[i] < num)
            {
                i++;
            }

            a[j] = a[i];
        }

        a[i] = num;
        QuickSort(a, start, i - 1);
        QuickSort(a, i + 1, end);
    }

Though it is character-building to transform a recursive algorithm into an iterative form (and vice versa) I would not worry too much about iterative vs recursive.

There are several ways you could improve this code.

As I said in your previous post: why an array? You could be much more general and sort an IList<int>.  And why ints?  You can sort any collection of things that can be compared consistently which would make your sorting algorithm more useful.
start and end are very clear. i, j and num are not.  How is the reader of this code supposed to understand what it does?  Rename num to what it is: the pivot.
Recursive quicksort has four basic steps: (1) decide if we're already sorted, (2) choose a pivot, (3) partition and (4) recurse. The considerable majority of your algorithm is devoted to (3). Consider putting the partition logic in a helper method that can be clearly shown to be correct.
Consider showing us your test cases.
There is no error handling; what if the array is null?  What if the start and end are out of bounds?  And so on.
Consider adding postcondition assertions.  A postcondition assertion is a Debug.Assert that documents what the method ensures is true just before it returns. In your case the postcondition is "the array is sorted from start to end". So assert that; write a little "is sorted" predicate and verify that it works. This will help you find bugs, if there are any. This will help future readers of the code understand it. And it will help people who change the code in the future understand what needs to not break when they modify the code.
The partition step also has a postcondition; after the partition the array is partitioned into three parts: the values before the pivot are smaller or equal to it, the pivot, and the values after the pivot are greater than it. Assert that these conditions are met.


Cut float numbers in a stream (putchar, getchar) and in a string For example, the following row:

"a+12.3456-b-0.456789+1.3-45678"

will be transformed into

"a+12.34-b-0.45+1.3-45678"

That is, we need to cut out the figures after the point from the third figure. The signal of the row's end is the sign '&'.

With the getchar and putchar functions:

char c = 0, c_ = 0,flag=0;
while ((c = getchar()) != '&')
{

    if (isdigit(c) && !(isdigit(c_)))
    {
        flag = 1;
    }

    putchar(c);

    if (flag)
    {
        while ((c = getchar()) != '.') putchar(c);

        putchar(c);

        c = getchar();
        putchar(c);
        if (!isdigit(c)){ flag = 0; continue; }

        c = getchar();
        putchar(c);
        if (!isdigit(c)){ flag = 0; continue; }

        while (isdigit(c=getchar()));
        putchar(c);
        flag = 0;
    }
    c_ = c;
}

Using strings:

char s[100] = { 0 }, s1[100] = { 0 };
inti = 0, j = 0;

gets(s);

while (s[i] != '&')
{
    if (isdigit(s[i])
    {
        while (s[i] != '.')
        {
            s1[j++] = s[i++];
        }

        s1[j++] = s[i];

        s1[j++] = s[++i];
        if (!isdigit(s[++i])) continue;

        s1[j++] = s[i];
        if (!isdigit(s[++i])) continue;

        while (isdigit(s[++i]));

        continue;

    }

    s1[j++] = s[i++];
}
puts(s1);

The programs are confusing and ugly, but they're the best ones I ended up with. Please suggest a better algorithm or change mine to help it make more sense.

For input/output, it's generally better to work with strings rather than one character at a time with getchar() and putchar() — that is, if you can manage to write the code elegantly.  String-processing code would be more reusable, and it would likely avoid the overhead of repeatedly calling getchar() and putchar().

Note, however, that gets() is a deprecated function that you should never use because it offers no way to protect against buffer overflow.  Instead, use fgets() (which lets you specify a buffer size) or getline() (which automatically resizes the buffer).

Neither implementation is easy to follow.

The first version has calls to getchar(), putchar(), and isdigit() all over the place, as well as nested loops and mysterious flag variable.  Only the outer loop test for the '&' terminator character, which seems wrong.

The second version increments i and j all over the place, and has nested loops (of which only the outer one tests for '&').  It's clear that the output string will be no longer than the input, so it should be possible to reuse the original s buffer.

A better approach would be to have one loop that looks at each input character in turn.  The loop copies one character of input to the output per iteration, except when excess decimal precision is seen.  Instead of s[i] and s1[j], it would be easier to use char pointers named in and out.

/**
 * Overwrites string s with a string in which numbers are truncated after
 * some number of decimal places.  The input s may be terminated by a NUL
 * or an ampersand character.
 */
char *truncate_decimals(char *s, int max_places)
{
    int places = -1;
    char *out = s;
    for (char *in = s; *in != '\0' && *in != '&'; in++)
    {
        if (*in == '.')
        {
            places = 0;   // Decimal point encountered. Start counting places
        }
        else if (!isdigit(*in))
        {
            places = -1;  // Non-digit encountered. Resume normal copying
        }
        else if (places >= 0 && ++places > max_places)
        {
            continue;     // Excess precision; don't copy
        }
        *out++ = *in;
    }
    *out = '\0';
    return s;
}


Hide rows where column F is not empty I'm not a scripting expert by any means, but I put together some code that I needed for Google Sheets. The good news is that it actually works!

However, it's noticeably slow to run (about 15 seconds).  I'm sure it's not optimized to deal with arrays and other fun things that I don't fully understand...

function ShowHideClosedCards() {
  var ss = SpreadsheetApp.getActiveSpreadsheet();
  var sheet = ss.getActiveSheet();
  var lastRow = sheet.getLastRow();

  strVisible = sheet.getRange('N1').getValue(); //Get the value of N1
  if (strVisible == '** Closed accounts are visible **') { //If the closed accounts are visible...
      for( i=6 ; i<=lastRow ; i++) { // Start with row 6 and continue through the last row: i <= lastRow
       var status = sheet.getRange("F"+i).getValue(); //Get the value of the cell.
       if (status !== "") { // If there's something in the "Date Closed" cell, then that should mean it's closed
         sheet.hideRows(i); // Hide the row
       }
       }
       sheet.getRange('N1').setValue('** Closed accounts are hidden **');
  } else {  // Otherwise, assume the closed accounts are hidden...
       for( i=6 ; i<=lastRow ; i++) { // Start with row 6 and continue through the last row: i <= lastRow
       var status = sheet.getRange("F"+i).getValue(); //Get the value of the cell.
       if (status !== "") { // If there's something in the "Date Closed" cell, then that should mean it's closed
         sheet.showRows(i); // Show the row
       }
       }
       sheet.getRange('N1').setValue('** Closed accounts are visible **');
  }
}

The gist of it is that it should go through each row (from 6 through 500) and if there's anything in column F of that row, hide the row.  Then it marks one cell (N1) to let the user know that the closed accounts are hidden.

If the user runs the code again, it should do the same thing, but unhide those rows (and only those rows).  Then it marks N1 again to let the user know that the closed accounts are visible.

Any suggestions on how to get it to run faster?

In order to improve the performance of your script you should read cell values from an array that represents the entire range that your are going to scan.

This means that instead of repetedly calling

var status = sheet.getRange("F"+i).getValue(); //Get the value of the cell.

you should get everything in a single call like this one

var values = getRange(row, column, numRows, numColumns).getValues();

it'll give you an array that you can iterate very fast. Example:

for (var row in values) {
  for (var col in values[row]) {
      ...
  }
}

Hiding rows can be a little bit tricky if you want to optimize it becasue it requries some additional work. If you do it for each row separately then you'll loose some performance agian. This means that this is not good:

sheet.hideRows(i); // Hide the row

In my scripts that work with a few hundereds or thousands rows hiding some of them lasts no longer than 1-2 seconds.

Here's how it goes:

I have a special column that I call Visible (or whatever). I use it to store a boolean value.

When I run a loop I store the flag in a second array like var visibles = []; instead of calling hideRows each time.

After the loop is finished I update the Visible column with:

// update row visibility
var visibleRange = sheet.getRange(row, column, numRows, numColumns);
visibleRange.setValues(visibles);

This runs super fast.

Next you have to unhide all rows with:

// show all rows
sheet.showRows(rowIndex, numRows);

You have to sort them by visibility:

// sort by 'visible' and make 'true' (visible) rows first
sheet.sort(..., false);

Sorting is necessary so that you can finally hide an entire range in a blink of an eye:

// hide 'false' (not-visible) rows
var rangeToHide = sheet.getRange(.., .., ..);
SpreadsheetApp.getActiveSpreadsheet().hideRow(rangeToHide);

If you need a different order you can sort it again after hiding.

With this solution it requires just a couple of calls which will make your script really smooth.

You can read about a couople more optimization tipps on Google Apps Script - Best Practices


Taking the sum and average of the keyboard inputs I have an assignment as follows:

  Write a program where you can enter from the keyboard up to 10
  integers. If the number entered is equal to -99, stop reading numbers
  from the keyboard and compute the average and sum of all values
  (excluding -99). Print the average on the screen. Make sure you
  consider all the cases: less than 10 or exactly 10 numbers can be
  entered. After the numbers have been entered you need to make sure
  that the average is computed then.

I came up with the program below. Do you think my program is fine? What could be alternative solutions?

s = 0
for i in range(1, 11):
    a=int(input("Enter a number: "))
    if a==-99:
        s = s+a
        print("The sum of the numbers that you've entered excluding -99:",s-a) # minus a because we want to exclude -99
        print("The average of the numbers that you've entered excluding -99:",(s-a)/(i-1)) # minus a because we want to exclude -99. i-1 in denominator because we excluded -99 from the sum.
        break
    else:
        s = s+a
        print("The sum of the number(s) that you've entered:",s)
        print("The average of the number(s) that you've entered:",s/i)
        continue
input("Press enter to close")

A few things:

The application should do 2 things.

  Collect up to 10 different integers, or until the entered value is -99
  Sum and Average the values

# so lets start by creating a list and fill it with all the numbers we need
numbers = list()
for i in range(0, 10):
    inputNr = int(input("Enter a number: "))
    if(inputNr == -99):
        break;

    numbers.append(inputNr)

#Then we take all of the numbers and calculate the sum and avg on them
sum = 0
for j, val in enumerate(numbers):
    sum += val

print("The total sum is: " + str(sum))
print("The avg is: " + str(sum / len(numbers)))

A few pointers about your code:

Use a whitespace (a single space) between variable declaration and assignment. Where you did a=1 you want to do a = 1
You usually iterate an array (or in Pythons case a list) from 0 to N, and not 1 to N
Create variables which you can reuse. As you can see in my example I create a inputNr variable which holds the input. I later reuse this to check if it's -99 and I reuse it by adding it to my list
variable names, function names, and everything else should have a descriptive and concise name. Where you named your input variable to a I named mine to inputNr. This means that when we later reuse this variable in the code it will be clear to us what it is and what it holds.


Finding a common prefix/suffix in a list/tuple of strings The question that sparked this question, was one on Stack Overflow in which the OP was looking for a way to find a common prefix among file names( a list of strings). While an answer was given that said to use something from the os library, I began to wonder how one might implement a common_prefix function.

I deiced to try my hand at finding out, and along with creating a common_prefix function, I also created a common_suffix function. After verifying that the functions worked, I deiced to go the extra mile; I documented my functions and made them into a package of sorts, as I'm sure they will come in handy later.

But before sealing up the package for good, I deiced I would try to make my code as "Pythonic" as possible, which lead me here.

I made sure to document my code heavily, so I feel confident that I shouldn't have to explain how the functions work, and how to use them:

from itertools import zip_longest

def all_same(items: (tuple, list, str)) -> bool:
    '''
    A helper function to test if
    all items in the given iterable
    are identical.

    Arguments:
    item -> the given iterable to be used

    eg.
    >>> all_same([1, 1, 1])
    True
    >>> all_same([1, 1, 2])
    False
    >>> all_same((1, 1, 1))
    True
    >> all_same((1, 1, 2))
    False
    >>> all_same("111")
    True
    >>> all_same("112")
    False
    '''
    return all(item == items[0] for item in items)

def common_prefix(strings: (list, tuple), _min: int=0, _max: int=100) -> str:
    '''
    Given a list or tuple of strings, find the common prefix
    among them. If a common prefix is not found, an empty string
    will be returned.

    Arguments:
    strings -> the string list or tuple to
    be used.

    _min, _max - > If a common prefix is  found,
    Its length will be tested against the range _min
    and _max. If its length is not in the range, and
    empty string will be returned, otherwise the prefix
    is returned

    eg.
    >>> common_prefix(['hello', 'hemp', 'he'])
    'he'
    >>> common_prefix(('foobar', 'foobaz', 'foobam'))
    'foo'
    >>> common_prefix(['foobar', 'foobaz', 'doobam'])
    ''
    '''
    prefix = ""
    for tup in zip_longest(*strings):
        if all_same(tup):
            prefix += tup[0]
        else:
            if _min <= len(prefix) <= _max:
                return prefix
            else:
                return ''

def common_suffix(strings: (list, tuple), _min: int=0, _max: int=100) -> str:
    '''
    Given a list or tuple of strings, find the common suffix
    among them. If a common suffix is not found, an empty string
    will be returned.

    Arguments:
    strings -> the string list or tuple to
    be used.

    _min, _max - > If a common suffix is  found,
    Its length will be tested against the range _min
    and _max. If its length is not in the range, and
    empty string will be returned, otherwise the suffix
    is returned

    eg.
    >>> common_suffix([rhyme', 'time', 'mime'])
    'me'
    >>> common_suffix(('boo', 'foo', 'goo'))
    'oo'
    >>> common_suffix(['boo', 'foo', 'goz'])
    ''
    '''
    suffix = ""
    strings = [string[::-1] for string in strings]
    for tup in zip_longest(*strings):
        if all_same(tup):
            suffix += tup[0]
        else:
            if _min <= len(suffix) <= _max:
                return suffix[::-1]
            else:
                return ''

common_suffix can be written as return common_prefix(string[::-1])[::-1] because the operations are just the simmetric of one another, and this way will prevent duplication.

Also I think you should not handle max or min inside the common_prefix function because it feels like the function has double responsabilty: finding prefixes + length interval check.

Why are you limiting yourself to strings? Python allows general functions very easily.

Why do you build all the result and then return it? You should yield the result item by item:

Why do you write so much yourself? Using the itertools module is much more efficient and simple:

def common_prefix(its):
    yield from itertools.takewhile(all_equal, zip(*its))

PS: common_suffix will now need to use reversed(list instead of [::-1]


Reading a list of file paths and adding them to a map I have many thousands of image files spread around various places and would like to copy them into one directory. However, may of the files are duplicates with the same names (often thumbnails or reduced resolution etc).  I want to delete all files that have the same name except for the largest.

The following code reads a list of these file paths (fullpath/name). It splits the filename from the path, determines the size of the file and adds each filename and its size to a list (map).  Where a name is found that is already in the list, the larger of the two is kept in the list and the smaller is 'deleted' (by which I mean emitting the command "rm path/to/file").

I know this is naive C++, because I am a novice (though not at C). Any help making it less naive would (or just better) is appreciated.

#include <iostream>
#include <string>
#include <sys/stat.h>
#include <map>

using std::map;
using std::string;

class filedata {
    std::string path;
    size_t size;
public:
    filedata (void) : path(""), size(0) { }
    filedata (const string& p, size_t s) : path(p), size(s){ }
    filedata (const filedata& f) : path(f.path), size(f.size) { }

    size_t filesize(void) {
        return size;
    }

    string& filepath(void) {
        return path;
    }

    std::ostream& print(std::ostream& os) const {
        os << size << " " << path << "\n";
        return os;
    }
};

std::ostream& operator<<(std::ostream& os, const filedata& f)
{
    return f.print(os);
}

static size_t file_size(const string& s)
{
    struct stat st;
    if (stat(s.c_str(), &st) == 0) {
        return st.st_size;
    }
    return 0;
}

static void delete_file(const string& s)
{
    std::cerr << "rm \"" << s << "\"\n";
}

static bool file_name(const string& path, string& name)
{
    size_t pos = path.rfind('/');
    if (pos == string::npos) {
        return false;
    }
    name = path.substr(pos+1);
    return true;
}

int main(int argc, char **argv)
{
    (void) argc;
    (void) argv;

    string s;
    map<string, filedata> files;

    while (getline(std::cin, s)) {
        string name;
        if (file_name(s, name) == false) {
            break;
        }

        size_t size = file_size(s);
        if (size == 0) {
            delete_file(s);
        }
        else if (files.count(name) > 0) {
            filedata f(files[name]);
            if (f.filesize() <= size) {
                delete_file(f.filepath());
                files[name] = filedata(s, size);
            }
        }
        else {
            files[name]= filedata(s, size);
        }
    }

    for (auto i = files.begin(); i != files.end(); ++i) {
        std::cout << i->first << " "  << i->second;
    }
    return 0;
}

Looks good overall (though I'm by no means a C++ expert).

In coming brain-dump though:

void argument lists are rare in C++. Yes, in C, int c() and int c(void) are different, but in C++, that's not the case. In C++, int c() is implicitly the same declaration as int c(void).

The no-op statements on argv and argc jump out as odd. If you did that so a warning of an unused variable won't be issued, you could always use one of the alternate main declarations (i.e. int main().

Opinion again, but I'm not a fan of comparison to booleans: if (file_name(s, name) == false).  It reads more easily as if (!file_name(s, name))

This might be on purpose, but the path property of filedata can be mutated from outside:

filedata f("name", 10);
f.filepath() = "new";

I would probably make these objects immutable since they seem to be value objects (I think you probably intended to do this, just non-intuitive references got you).

You should return either by value or by const reference from filepath. Even though it will create a usually unnecessary copy, I highly suggest the value route. If you return a const reference, you run the risk of a dangling pointer. Consider this (bad) example:

filepath* f = new filepath("blah", 10);
const std::string& s = f->filepath();
delete f;
// Any use of s past here is undefined behavior

s is essentially a const pointer to f->path. This means that when f is released, s becomes invalid. Note that if you immediately create a copy of a string based on the constant reference [i.e. if you had std::string s = ...] this does not apply. That still creates a copy though, so you might as well return a value.

(Also, obligatory note: raw pointers have very few legitimate uses in C++. Avoid pointers if at all possible, and use a smart pointer when not. The only (possible) exception is low level container implementations.)

I'm not a fan of print style member functions.  In this application this aversion doesn't make sense, but imagine if some third party library you used provided print methods for some of it's classes.  Do you think it would print out in exactly the format you want?  I prefer to use helper-type functions for printing rather than putting it as a member of the class.  (Though once again, on this application since it's so small, it matters nothing at all.)

(Oh, and this once again is opinion.)

filedata (void) : path(""), size(0) { }
filedata (const string& p, size_t s) : path(p), size(s){ }
filedata (const filedata& f) : path(f.path), size(f.size) { }

There's a bit of redundancy here.  Technically you could write this just as:

filedata (const string& p = "", size_t s = 0) : path(p), size(s){ }

Or if you wanted:

filedata (const string& p = string(), size_t s = size_t()) : path(p), size(s){ }

Though I'd probably go with the first.

The copy constructor doesn't need to be defined since it's the same as the implicit one. When using the same functionality as the implicit one, do not define one. The implicit one isn't prone to typos or a future failure to update the copier to reflect new properties.

I would give more descriptive names to p and s:

filedata (const string& path, size_t size) : path(path), size(size) { }

I'd probably go with something like uint64_t instead of size_t since size_t will probably be 32 bits on a 32 bit system. If you'd like to abstract away exactly what the type is, you could have a typedef ... size_type member of your class. Not only does that offer a more semantic meaning, it allows for easier future change (assuming consumers actually proper use the typedef...).

I tend to declare define the end of an iterator up front:

for (auto i = files.begin(), end = files.end(); i != end; ++i) {

Not only does it potentially offer a tiny performance advantage (though not really on any modern compiler), it has clearer meaning. With this version, there much more suggestion that end is not mutated inside of the loop (though it's still not guaranteed since end isn't const).

std::begin and std::end should be preferred in C++11. They have all the capabilities of the member-function versions plus a bit more.

stat failing probably shouldn't just return a size of 0.  You might end up accidentally deleting a file if something weird happens.  (Then again, most of the reasons stat would fail would also cause an attempt to delete a file to fail.)

If you had the input include the file sizes, you could reduce your program to pure text processing.  I'm also not sure if your program should be emitting rm commands.  Seems like a coupling of sorts.  Then again, the output would just be transformed into rm commands anyway, so might as well do it directly I guess :).

The data put out on stdout seems to be debugging-esque information whereas the stderr data seems to be what you're actually after.  Seems like these streams should be swapped.

I'd be tempted to use a reference instead of copying the item in the map:

filedata f(files[name]);

Could be:

filedata& f = files[name];

The performance difference is going to be non-existent; mostly just a (opinion-y) style thing. It would also allow you to simplify the reassignment from files[name] = ... to just f = ....

The filesize member of filedata can't permute size, so you might as well have the method be const.  Same for filepath() if it wasn't meant to allow the perumtation of path (as mentioned earlier).

Another personal-style thing: I don't like implicit visibility scoping:

class filedata {
    std::string path;
    size_t size;

os << size << " " << path << "\n";
return os;

Can be simplified to:

return os << size << " " << path << "\n";


Evaluating an expression with integers, +, and *, as well as -, / There is a job interview question, and the source of the question is here.

The solution is pretty simple. We just need to split the input string by + and then by *. Then we compute products in a nested loop and sum up all the products.

public class Main {

    // http://www.careercup.com/question?id=4911380140392448
    public static void main(String[] args) {
        String equation = "1*5*4+8*9+16";
        int result = compute(equation);
        System.out.println(result);
    }

    static int compute(String equation) {
        int result = 0;
        String []byPluses = equation.split("\\+");
        for (String multipl : byPluses) {
            String []byMultipl = multipl.split("\\*");
            int multiplResult = 1;
            for (String operand : byMultipl) {
                multiplResult *= Integer.parseInt(operand);
            }
            result += multiplResult;
        }
        return result;
    }

}

But what if I was asked the following question after solving the initial problem:

  OK. What would you do if you needed to support not only + and * but also - and /?

The quick solution I thought up is this:

package careepcup.fb;

public class Main {

    // http://www.careercup.com/question?id=4911380140392448
    public static void main(String[] args) {
        // An equation with +, -, /, *
        String anotherEquation = "1*5*4+8*9+16/8-9"; // 85
        double another = computeAnother(anotherEquation);
        System.out.println(another);
    }

    static double computeAnother(String equation) {
        double result = 0.0;
        String noMinus = equation.replace("-", "+-");
        String[] byPluses = noMinus.split("\\+");

        for (String multipl : byPluses) {
            String[] byMultipl = multipl.split("\\*");
            double multiplResult = 1.0;
            for (String operand : byMultipl) {
                if (operand.contains("/")) {
                    String[] division = operand.split("\\/");
                    double divident = Double.parseDouble(division[0]);
                    for (int i = 1; i < division.length; i++) {
                        divident /= Double.parseDouble(division[i]);
                    }
                    multiplResult *= divident;
                } else {
                    multiplResult *= Double.parseDouble(operand);
                }
            }
            result += multiplResult;
        }

        return result;
    }
}

What do think of my solution with -, +, /, *?

@tim has covered a bunch of what I was going to add, but here are some additional points:

You should anticipate having whitespace in the input. A "simple" replace-all would suffice:

equation = equation.replaceAll("\\s+", "");

Your solution of converting - to +- is novel, and effective, but it really should have a comment on it explaining that you are going to re-split the String and parse the new - as part of the integer, rather than as an operator. Leaving it as it is required some detective work on how it functions.

Bug: Your system will fail on things like: 1 + 2 * -3  (ignoring the whitespace).

Your inner workings of the * and / is a bit messy, though. A logical progression for your challenge would be to add, say, a % operator.... which would require a complicated change.

Operators of equal precedence should be handled together. treating * specially when compared to / makes it awkward because you have to test all combinations inside the * breakdown....

I would suggest using a smarter split expression, one that splits on the gaps between the operators and the values...:

String[] parts = operand.split("(?=[/*])|(?<=[/*])");

With the above regex, you will get for example:

1/2*773 -> [1, /, 2, *, 773]

The way the regex works is that it looks for two things (the regex is in two parts - using "lookaround" expressions)....

(?=[/*]) - A positive zero-width look-ahead - This says: find any gap between two characters where the next character is a / or *.
(?<=[/*]) - A positive zero-width look-behind - This says: *find any gap between two characters where the previous character was a / or *.

Put them together with an or condition, it says: split the input on the gaps before and after / or *.
Now, you can just initialize the result to index-1, and then loop through the rest.... :

double result = Double.parseDouble(parts[0]);

for (int i = 1; i < parts.length; i += 2) {
    String op = parts[i];
    double val = Double.parseDouble(parts[i+1]);
    switch (op) {
        case "*" :
            result *= val;
            break;
        case "/" :
            result /= val;
            break;
    }
}
return result;


Showing the path of a projectile I'm trying to dive deeper into C#, and so I've been experimenting with some code. I'm creating a console application that shows the path of a projectile depending on the angle, and it works great! You can check it out here actually. It's not perfect, but I like it.

Is there a better way to use my for loops? I'm using 3 inside eachother, and I'm not sure if that should be an issue. If you could take a look at the code in the link, it'd be great.

I realized my code might not have shown up. The code snippet won't work if you try to do it as you're missing a couple functions I've devised, but you should be able to get the idea just by looking at it.

using System.IO;
using System;
using System.Threading;
class Program
{

static void Main()
{
    Console.Clear();

    DoArc();
}

static void DoArc() {

            int tempVal = 0;
            string displayLine = "";
    //HERE
    for(int a = 0; a < 90; a+= 2) {

        Console.Clear();
        //HERE
        for(int y = 0; y < 16; y++) {
            //HERE
            for(int x = 0; x < 40; x++) {
                tempVal = Mathf.RoundToInt(Physics.HeightAtPoint(x, a, 15f, 0));
                tempVal = Mathf.Clamp(tempVal, 0, 16);

                if(Mathf.CloseTo(16 - y, tempVal, 1))
                displayLine += " X";
                else
                displayLine += " .";

            }
        Console.WriteLine(displayLine);
        displayLine = "";
        }
        Console.WriteLine("Angle: " + a);
        Thread.Sleep(500);

    }
}
}

I don't see anything particularly wrong with the loops themselves, but, as far as coding style goes:

tempVal and displayLine should be declared as close as possible to the place where they're first used.
the values returned by Math.RoundToInt and Mathf.Clamp don't seem to have anything in common, so they should be assigned to different variables. Don't reuse tempVal for different and unrelated things.
tempVal, x, y and a should be given more meaningful names.
You should use a StringBuilder to build strings, instead of using string concatenation.
Your indentation is all over the place.
In C#, it is common practice to put the opening bracket { on its own line - although one could argue that this is a matter of personal preference and doesn't really matter as long as you're consistent throughout the codebase.

for(int y = 0; y < 16; y++)
{
    var sb = new StringBuilder();

    //HERE
    for(int x = 0; x < 40; x++)
    {
        int height = Mathf.RoundToInt(Physics.HeightAtPoint(x, a, 15f, 0));
        int clamp = Mathf.Clamp(height, 0, 16);

        if(Mathf.CloseTo(16 - y, clamp, 1))
            sb.Append(" X");
        else
            sb.Append(" .");

    }
    Console.WriteLine(sb.ToString());
}


C++ Hashing Passwords - simple algorithm using rand() I'm studying algorithm's and got to the point of hashing.I was excited about it and I created a little hashing function which will depend on user input and generate passwords. The passwords had to look completely random, depend largely on input and be consistent(same input - same pass generated), however not looking similar for similar inputs.I have read someware that function rand() in C++ will generate same sequence of random numbers for same seed. So I used it as a part of algorithm. I'm not sure how safe it is but so far it works well. Any ideas on improving code or algorithm are welcomed!

void User::HashPassword()//Hash pass method of User class
{
    if (IsExisting())//checking if input was valid
    {
        UserHashPassword.clear();
        std::string Combine = UserName + UserPassword + UserURL;
        const unsigned int VALUE = Combine.length();
        for (auto Letter : Combine)
        {
            srand(VALUE*Letter);//seed that depends on user input - new for each letter, and input total length
            UserHashPassword += 33 + rand() % 92;//borders set for ASCII codes that exist on english keyboards
        }
    }

}

Structure
Okay, let's start with the basic overall structure of the function: it's, frankly, not very good. It (unnecessarily) depends on a number of outside variables--from its perspective, essentially globals.
There's no good reason that this couldn't be a nice, pure function--i.e., take an input parameter, do the hash, return a result. No reference to anything outside the function at all.
int HashPassword(std::string const &Combine) {
    unsigned int hash = 0;

    const unsigned int VALUE = Combine.length();
    for (auto Letter : Combine)
    {
        srand(VALUE*Letter);
        hash += 33 + rand() % 92;
    }
    return hash;
}

Naming
Some of the names are pretty good (e.g., Letter, HashPassword) but others (e.g., VALUE, Combine) seem like they're open to some improvement. I can see how you came up with Combine, but that doesn't really make it a good name. VALUE is so generic that it's probably best to avoid it except (perhaps) in something like a class that represents some number, where the VALUE of the object has a fairly obvious meaning.
Magic numbers
The 33 and 92 are a couple of examples of magic numbers. These generally lead to code that's difficult to read (at best).
Security
This hash function provides approximately what's usually referred to as "kid sister" level security. If you have a kid sister, this might be about enough to keep her fooled--but only if she's not very bright or fairly quickly bored.
The most obvious problem is that this just generates a value from each byte, then adds those values together. This means nobody really needs to know your actual password--at most they only need to care about the letters that make up the password. For example, HashPassword("ABC"), and HashPassword("CAB") both produce exactly the same result.
So, to get the right result, we don't really even need to guess their password--we just need to guess some characters that produce the right result--and since nearly every input in the right range maps to a unique result, it doesn't take a lot to come up with a usable result either. For example, let's consider a result of 628. Each input produces a result from 32 to 124, so we might want to use, say, seven characters in a password. So, let's start by computing the result for seven repetitions of each character in the most common range:
for (int i = '0'; i < 'z'; i++) {
    std::string in(7, (char)i);
    std::cout << "in: " << in[0] << ", hash: " << HashPassword(in) / 7 << "\n";
}

So, our smallest result is 32--a little more than the "28" that's left over if we divide 628 by 6, so we want something that produces a result a little less than 100. A quick glance shows that 1 produces a result of 87--that'll probably do. So, if we start with 111111, we should get a result of 6 * 87 = 522. That gives us 106 left over. Another glance through the table shows that "R" gives 106, so an input of 111111R should give us 628. A quick run with     std::cout << HashPassword("111111R"); verifies that it does exactly that.
In case it seems like I hand-picked something that would be easy, I'll point out that I actually got the 628 from hashing "Password"--so to find 111111R, I really did start over from scratch--and I didn't even have to work hard at it--my first attempt worked out easily.
For another approach, I could have started with 5 of something. Let's say 'j', which gives 102. 5 x 102 = 510. I want two more characters to make up the remainder, so for my next character I want to pick something fairly small. Let's say _, which produces 35. With the previous 510, we get 545. A quick subtraction shows that we need 83 more, and a quick search shows that "M" produces 83, so jjjjj_M would work as well (and a quick test verifies that yes, it also produces 628).
Bottom line: from a security viewpoint, this is so weak that it's open to question whether it really even qualifies as "kid sister" level, or if it's too weak to even qualify for that level of "security".


Check existence of matching key,value pair I recently was in an interview where I was asked to implement a snippet that finds the matching key:value pair among two JavaScript arrays in which each member is an object:

var a = [{color:'blue'},{color:'green'}, {color:'brown'}, {color:'black'}]
var b = [{color:'white'},{color:'yellow'}, {color:'black'}, {color:'blue'}]
var expectedOutput = [{color:'blue'}, {color:'black'}]

Here's my implementation:

var a = [{color:'blue'},{color:'green'}, {color:'brown'}, {color:'black'}]
var b = [{color:'white'},{color:'yellow'}, {color:'black'}, {color:'blue'}]
var expectedOutput = [];
for(var i=0; i<a.length; i++){
 if(b[i].hasOwnProperty('color') && a[i].color===b[i].color){
  expectedOutput.push(a[i]);
 }
}

document.write(expectedOutput.toString());

In the interview, I communicated that using .hasOwnProperty() provides \$O(1)\$ lookup for keys (but not for values) from array b, and unfortunately, I wasn't able to fully implement this as my mind wandered across other possibilities there might be, not to mention the pressure.

I believe this still has \$O(n^2)\$ complexity as it simply checks the existence of child .color key as well as a supplementary check against values being equivalent. All in all, I suspect the interviewer was pointing me into this implementation for \$O(n)\$ complexity.

Do you think otherwise? If you do, could you kindly explain why and how? If my question is unclear, how would you solve it in \$O(n)\$ time in JavaScript?

I would say that hasOwnProperty is not necessary in this case. If you want to make it O(n) you probably have to first convert one of the arrays into a hash of colors so that you can improve lookup speed. Something like:

var a = [{color:'blue'},{color:'green'}, {color:'brown'}, {color:'black'}];
var b = [{color:'white'},{color:'yellow'}, {color:'black'}, {color:'blue'}];
var aColors = a.reduce( function(obj, hash) { obj[hash.color] = true; return obj }, {} );

var expectedOutput = [];
for(var i=0; i<b.length; i++) {
 if (aColors[ b[i].color ] )
  expectedOutput.push( b[i] );
}

console.log(expectedOutput);


Handling C-like strings in C++11 We have two C-strings and for their concatenation we need to know the size of resulting string. And we want to calculate it at compilation stage. How can I improve this solution?

char s1[] = "string1";
char s2[] = "string2";
constexpr auto SIZE =
sizeof(s1) / sizeof(*s1) + sizeof(s2) / sizeof(*s2) + 2; // + for space and \0
char ss[SIZE];

This is one case where using the preprocessor may be worthwhile (despite the lack of namespace awareness).  In the code below, ss[] not only gets an appropriate size without any effort on your part, it's actually populated with the concatenated text at compile time.

#define S1 "string1"
#define S2 "string2"

// optionally, create real character arrays so you can modify them etc.
char s1[] = S1;
char s2[] = S2;

// now the magic - automatic concatenation of adjacent string literals
char ss[] = S1 S2;


Finding the node with the smallest maximum distance to all the other nodes This code is supposed to find, in a graph, the node with the smallest maximum distance to all the other nodes. The problem from Quora is :

  How do I find the node that has the least maximum distance to all the
  other nodes in a tree? "So you're thinking about moving into an
  unknown city. You have been given a map of the city and want to get a
  house by the street corner that has the least maximum distance to all
  the other street corners. (Think the street corners as nodes, and the
  way between them as edges.)"

For instance, in this example graph:

2 is the node that has the least maximum distance to all other nodes. I found out that the complexity is \$O(n*(n*m))\$, and that works great when I have less than 5000 nodes. What could I possibly fix to make this solution handle nodes >100 000? Should I use, for instance, iteration instead of recursion?

I have done some "minor" tweaks, replacing all cout and cin With printf and scanf, and decreased the execution time by 40ms.

#include <cstudio>
#include <queue>
#include <vector>
int biggest_dist(int n, int v, const vector< vector<int> >& graph)
{ //This function finds the diameter of thegraph
int INF = 2 * graph.size(); // Bigger than any other length
vector<int> dist(n, INF);

dist[v] = 0;
queue<int> next;
next.push(v);

int bdist = 0; //biggest distance
while (!next.empty()) {
    int pos = next.front();
    next.pop();
    bdist = dist[pos];

    for (int i = 0; i < graph[pos].size(); ++i) {
        int nghbr = graph[pos][i];
        if (dist[nghbr] > dist[pos] + 1) {
            dist[nghbr] = dist[pos] + 1;
            next.push(nghbr);
        }
    }
}

return bdist;
}

int main()
{
int N; scanf("%i", &N);
int minst;
vector< vector<int> > graph(N);
vector<int> noder;
for (int i = 0; i < N - 1; ++i) {
    int a, b;
    scanf("%d %d", &a, &b);

    graph[a].push_back(b);
    graph[b].push_back(a);
}

int answer = 0;
int dis = biggest_dist(N, 0,graph);
int maxdist;
for (int j = 0; j < N-1; ++j)
{
    maxdist = biggest_dist(N,j, graph);
    if (maxdist < dis)
    {
        dis = maxdist;
        answer = j;
    }
}
printf("%d", answer);

return 0;
}

It's not about constant optimizations. It's all about a more efficient algorithm. Here is a simple observation: the given graph is always a tree(because it has exactly n - 1 edges)(if it is not a tree, it is not connected and it is not clear what the answer is(maybe +infinity), so I will assume that it is a tree). That's why we can use the following algorithm:

Pick an arbitrary vertex v.
Find the furthest vertex from v(let's call it u).
Find the furthest vertex from u(let's call it t).
The distance between u and t is the answer.

This solution requires only two breadth-first searches so it has a linear time complexity. Why is it correct? I will not post a formal proof here, but drawing several trees on paper should help you to understand it. It is important that a given graph is a tree(this algorithm does not work for arbitrary graphs).

This algorithm actually finds a diameter of a tree, but it is easy to solve the original problem when a diameter is found. We can just pick the center of the diameter. To do it, we can find the path from u to t(it requires a small modification in bfs: we also need to keep track of a parent for each vertex) and store in a list. After that we just need to get the middle element of the list. If the number of elements in the list is even, we can pick any of two middle elements.


Ultimate functor class which can read/write to l-values/r-values The aim is to provide a generic functor class which makes references able to bind to r-values. The trick is to trick the compiler into thinking that the r-value is an l-value by using a forwarding references.
Here is the code:
#include <type_traits>
template <typename Functor>
struct Function {
    Functor func;
    template <typename ...Args>
    inline auto operator() (Args&&... args) {
        //I tried to provide a suitable error message if args were not valid parameters, but I can't get it to work...
        //static_assert(std::is_invocable_v<Functor, Args...>);
        return func(args...);
    }
};

Example use case:
# include <iostream>
# include <vector>
int main () {
    // Define function
    auto push_back = Function([] (std::vector<int>& vec, int num) {vec.push_back(num); return vec;});
    auto x = std::vector{1 , 2 ,3 , 4};
    // Works on l-values
    push_back(x, 5);
    std::cout << x[4] << '\n'; // 5
    // Works on r-values
    auto y = push_back(std::vector{1 ,2 , 3 , 4 , 5}, 6);
    std::cout << y[5] << '\n'; // 6
    return 0;
}

And finally, one can do this if you want to use old-style c functions:
template <typename R, typename ...Args>
inline auto CFunc(R (*iFunc) (Args...)) {
    return Function<R (*) (Args...)>(iFunc);
}

Is this robust code? Are there edge cases I need to be aware about with which my functor class does not work? Also, how could I provide more suitable error messages, eg when typename Functor is not a functor, or when args... are not suitable parameters?

Assuming C++20, we can constrain the template (similar to your attempt at static_assert).  The key is that Args... can contain rvalue-references, but we need to convert those to lvalue types, because that's what we present to the function:
#include <concepts>

    template<typename... Args>
    inline auto operator()(Args&&... args)
        requires std::invocable<Functor, std::add_lvalue_reference_t<Args>...>

I don't see anything else I would propose improving.


Calculate "average", "median" and "percentile" from a long array I have this method which calculates the average, median and percentile information given a long array of numbers. Is there any better and efficient way to do the same?

  public static double median(long[] numbers) {
    Arrays.sort(numbers, 0, numbers.length);
    int middle = numbers.length / 2;
    if (numbers.length % 2 == 1) {
      return numbers[middle];
    } else {
      return (numbers[middle - 1] + numbers[middle]) / 2.0;
    }
  }

  public static double average(long[] numbers) {
    int sum = 0;
    for (long l : numbers)
      sum += l;
    double average = 1.0d * sum / numbers.length;
    return average;
  }

  public static long[] percentiles(long[] numbers, double... percentiles) {
    Arrays.sort(numbers, 0, numbers.length);
    long[] values = new long[percentiles.length];
    for (int i = 0; i < percentiles.length; i++) {
      int index = (int) (percentiles[i] * numbers.length);
      values[i] = numbers[index];
    }
    return values;
  }

Validate your inputs

The median and average methods will throw an exception if the input array is empty. Think of the preconditions and if there are safe assumptions (input will never be empty) then document it, possibly with an assert statement, for example:

assert numbers.length > 0;

Note that the primary purpose of asset statements is documentation. Although a failed assert will crash the program, but only during development, as assert statements are removed from release builds, and so don't get executed in production. If you want to validate input, use exceptions instead.

Watch out for corner cases

This statement may overflow if the two numbers are too big:

return (numbers[middle - 1] + numbers[middle]) / 2.0;

To avoid overflow, you could divide each by 2 and then sum.

Simplify

If you omit the second and third parameters of Arrays.sort, by default it will use 0 and the length of the array. So you can omit these parameters in all your examples.

There is already a library method to calculate average:

return LongStream.of(numbers).average();


Toggling the visibility of drop-down menu items var x = 0;
document.getElementById("1").addEventListener("click", function() {
    if ((x % 2) == 0) {
        document.getElementById("dropdown_1").style.display = 'block';
    } else {
        document.getElementById("dropdown_1").style.display = 'none';
    }
    x += 1;
});
var y = 0;
document.getElementById("2").addEventListener("click", function() {
    if ((y % 2) == 0) {
        document.getElementById("dropdown_2").style.display = 'block';
    } else {
        document.getElementById("dropdown_2").style.display = 'none';
    }
    y += 1;
});

I made a HTML-Page with a menu that has a drop-down function. But i wasnt able to write it in a proper code, i thought using a loop would be helpful but i didnt manage to do so.

If you are ever writing an element ID that contains a number, as opposed to a totally unique name, you should re-think how you're using them. In fact, if you want to take a totalitarian mental shortcut, just never use IDs (at least, until you're very experienced and understand their pros/cons). Most JavaScript frameworks allow you to easily query for all elements that use a certain class, and even if you aren't using a framework, it's not so hard to loop through the results of a selector. That said, it's possible the main difficulty in this instance is associating a dropdown button with dropdown content. If the HTML is up for modifying, here's a loose way that I would do it.

HTML:

   <button type="button" class="openDropdown">Open</button>

CSS:

.dropdownContent:not(.visible) {
  display: none;
}

JS:

var dropdowns = document.querySelectorAll('.dropdown');
for (var i = 0; i < dropdowns.length; i++) {
  var dropdown = dropdowns[i];
  var content = dropdown.querySelector('.dropdownContent');
  dropdown.querySelector('.openDropdown').addEventListener('click', function() {
    content.classList.toggle('visible');
  }
}

Part of the reason I used classes as opposed to direct styling is it's easier to toggle; but also, the other options for display: properties can often conflict with a developer's attempt to show/hide it. inline-block is very useful, but has previously been messed up for me because of code that was showing/hiding an element and overrode that.

EDIT: RoToRa found a bug in the above code - the closure generation for the addEventListener block will preserve the variable content, but that variable will change before the loop actually finishes, so in the end it will only apply to the last instance. (You won't notice the issue if you only have one dropdown) Here's my attempt at a fix: Note the arguments to the listener, and the .bind at the end.

var dropdowns = document.querySelectorAll('.dropdown');
for (var i = 0; i < dropdowns.length; i++) {
  var dropdown = dropdowns[i];
  var content = dropdown.querySelector('.dropdownContent');
  dropdown.querySelector('.openDropdown').addEventListener('click', function(content, evt) {
    content.classList.toggle('visible');
  }.bind(null, content);
}

Closures are very convenient automatic ways to preserve a variable with asynchronous programming, but bind is a more manual way that treats it as an argument. Another option would have been to turn the entire inside of the loop into a function, and run it immediately (creating a separate closure context that will exist once for each loop run)


Leetcode: String to Integer (atoi) Solved this:

  Implement atoi to convert a string to an integer.

  Requirements for atoi:

  The function first discards as many whitespace characters as necessary
  until the first non-whitespace character is found. Then, starting from
  this character, takes an optional initial plus or minus sign followed
  by as many numerical digits as possible, and interprets them as a
  numerical value.

  The string can contain additional characters after those that form the
  integral number, which are ignored and have no effect on the behavior
  of this function.

  If the first sequence of non-whitespace characters in str is not a
  valid integral number, or if no such sequence exists because either
  str is empty or it contains only whitespace characters, no conversion
  is performed.

  If no valid conversion could be performed, a zero value is returned.
  If the correct value is out of the range of representable values,
  INT_MAX (2147483647) or INT_MIN (-2147483648) is returned.

Solution:

Check if string is null first then empty, if either is true, return 0. Keep iterating through string until we hit the first non-empty space character.

From this position, we check if the first character is '+' or '-'. If so, we move the iterator by one step and note the sign in boolean isNegative.

From the current position, as long as the character is an integer, we add it to solution while checking for overflow each time we do so. We keep iterating through the string until we hit a non-integer character and return solution:

class Solution {
    boolean isNegative = false;
    int i = 0; // string iterator position

    public int myAtoi(String str) {
        int solution = 0;

        if (str == null || str.isEmpty()) {
            return 0;
        }
        while (i < str.length() && str.charAt(i) == ' ') {
            i++; // discard leading whitespace
        }
        checkSign(str);
        for (; i < str.length(); i++) {
            if (str.charAt(i) >= '0' && str.charAt(i) <= '9') {
                int prev = solution; // keep solution from last iter to check for overflow
                solution = solution*10; // move number left one position
                solution = solution+(str.charAt(i)-'0'); // increase solution by curr integer
                if (isOverflow(prev, solution)) {
                    if (isNegative) {
                        return Integer.MIN_VALUE;
                    }
                    return Integer.MAX_VALUE;
                }
            } else {
                return signSolution(solution); // we've reached a non-integer character before end of string
            }
        }
        return signSolution(solution); // last character of string is an integer
    }

    boolean isOverflow(int prev, int curr) {
        // prev = value at last iteration
        // curr = value after current iteration
        if (curr/10 == prev) {
            return false;
        }
        return true;
    }

    void checkSign(String str) {
        if (str.charAt(i) == '+') {
            i++;
        } else if (str.charAt(i) == '-') {
            isNegative = true;
            i++;
        }
    }

    int signSolution(int solution) {
        if (isNegative) {
            return solution*-1;
        }
        return solution;
    }
  }

Normally when parsing a String into an int, people expect it to be a static method

Having to create a class to do the computation is a bit bulky, and then you need to worry about synchronization (what happens if this class is shared across multiple threads?)

boolean isOverflow(int prev, int curr) {
    // prev = value at last iteration
    // curr = value after current iteration
    if (curr/10 == prev) {
        return false;
    }
    return true;
}

This method is already a great candidate to become static, it only relies on its two inputs to figure out if an overflow happened

boolean isNegative = false;
int i = 0; // string iterator position

void checkSign(String str) //uses i and isNegative

int signSolution(int solution) //uses isNegative

These are slightly more tricky, int signSolution(int solution) can just take another parameter: static int signSolution(int solution, boolean isNegative)

void checkSign(String str) would need to be included as part of the myAtoI method or be rewritten a bit

If we treat + as whitespace, then it would only need to worry about handling -

static boolean checkSign(String str, int i)
{
    return str.charAt(i) == '-';
}

And calls to checkSign(str); would instead be if(checkSign(str)) i++;

Be consistent.  You're mixing for and while loops of similar structure

while (i < str.length() && str.charAt(i) == ' ')
    i++;
[...]
for (; i < str.length(); i++) {
    [...]
}

They can either both be for loops, or both be while loops, but mixing is unexpected

for(; i < str.length() && str.charAt(i) == ' '; i++);
[...]
for(; i < str.length(); i++)
{
    [...]
}

vs

while(i < str.length() && str.charAt(i) == ' ')
    i++;
[...]
while(i < str.length())
{
    [...]
    i++;
}

Checking "whitespace" currently only checks spaces

Instead of using str.charAt(i) == ' ', you can offload complicated functionality to another method (and be easier to change later)

static boolean isWhiteSpace(char c)
{
    return c == ' ';
}

Then if you'd like to include all whitespace (and + and mentioned above)

static boolean isWhiteSpace(char c)
{
    return Character.isWhitespace(c) || (c == '+');
}

Instances of if(bool) return true; else return false; are the same as just return bool;

if (curr/10 == prev) {
        return false;
}
return true;

Can be simplified to just

return curr / 10 != prev;

OptionalA:

Another way to check for overflow is to store the output as a long, and check if it overflows

public static int myAtoI(String str)
{
    long solution = 0;
    [...]
}

private static isOverflow(long num)
{
    return num == (int)num;
}

OptionalB:

Instead of adding the values and then flipping the result, you could subtract values if it's a negative number


A package for communications between packages - v2 This is basically the registry pattern and a pub/sub event system.

Very simple and minimalist.  Looking for general feedback.

/***************************************************************************************************
**COMMS
 - provides registry and event system
 - reduces dependencies
***************************************************************************************************/

// self used to hold client or server side global
(function (self) {

    "use strict";

    // holds (Pub)lic properties
    var Pub = {},

        // holds (Priv)ate properties
        Priv = {},

        // holds "imported" library properties
        $A;

    (function manageGlobal() {

        // Priv.g holds the single global variable, used to hold all packages
        Priv.g = '$A';

        if (self[Priv.g] && self[Priv.g].pack && self[Priv.g].pack.utility) {
            self[Priv.g].pack.comms = true;
            $A = self[Priv.g];
        } else {
            throw new Error("comms requires utility module");
        }
    }());

    Pub.Reg = (function () {
        var publik = {},
            register = {};
        publik.get = function (key) {
            return register[key];
        };
        publik.set = function (key, value) {
            register[key] = value;
        };
        publik.setMany = function (o) {
            $A.someKey(o, function (val, key) {
                register[key] = val;
            });
        };
        publik.getMany = function () {
            return register;
        };
        return publik;
    }());

    Pub.Event = (function () {
        var publik = {},
            events = {};
        publik.add = function (name, callback) {
            if (!events[name]) {
                events[name] = [];
            }
            events[name].push(callback);
        };
        publik.remove = function (name, callback) {
            if (name && callback) {
                delete events[name][callback];
            } else if (name) {
                delete events[name];
            }
        };
        publik.trigger = function (name) {
            if (events[name]) {
                $A.someIndex(events[name], function (val) {
                    val();
                });
            }
        };
        return publik;
    }());
    self[Priv.g] = $A.extendSafe(self[Priv.g], Pub);
}(this));

It looks good overall IMO. I won't comment on the bootstrap code as I'm not familiar with your library, but I would change a few things. Inside the closures that you create you could simply return the public object, without defining publik, it looks a bit cleaner. Then I added some annotation and changed the control flow a bit:

Pub.Reg = (function() {
  var register = {};
  return {
    get: function(key) {
      return register[key];
    },
    set: function(key, value) {
      register[key] = value;
    },
    setMany: function(o) {
      $A.someKey(o, function (val, key) {
        register[key] = val;
      });
    },
    getMany: function() {
      return register;
    }
  };
}());

Pub.Event = (function() {
  var events = {};
  return {
    add: function(name, callback) {
      // A bit more concise
      (events[name] = events[name]||[]).push(callback);
    },
    remove: function(name, callback) {
      // It's faster to set the value to `null`
      // than using the `delete` operator
      // but this depends on your use case;
      // the properties would still show up
      // in a `for..in` loop, but it's fine
      // if you use it merely as a dictionary
      if (!callback) {
        events[name] = null;
        return;
      }
      events[name][callback] = null;
    },
    trigger: function(name) {
      if (events[name]) {
        $A.someIndex(events[name], function (val) {
          val();
        });
      }
    }
  };
}());


Iterating through a list to generate calendars I'm iterating through a list to generate calendars, the issue is each generator function has the same this.addToCalendar() function. I'd like to know how I could condense this into only being called once, rather than in each function.
The main generators I'd like to look at are the ones within method, where I am generating calendar links as well as calling an external api.
CSS has been removed to focus on JS logic.
<template>

      <button @click="calendar.generator">
        {{ calendar.type }}
      </button>

  <button @click="setExpanded(false)">
    Close
  </button>

</template>

I'd like to know how I could condense this into only being called once, rather than in each function.

One option would be to make a method that accepted the calendar or type and called the associated method
methods:
    generateCalendar(type) {
        const generateMethod = `generate${type}Calendar`
        if (typeof this[generateMethod] === 'function')
          this[generateMethod]()
          this.addToCalendar()
        }
      }

Then pass the type:
<button @click="generateCalendar(calendar.type)">

If the only property being accessed in the loop was that type property then destructuring assignment would allow it to be simplified:

   <button @click="generateCalendar(type)">
      {{ calendar.type }}
    </button

Alternatively, with using the method above the generator properties of the calendars could be removed, and calendars could perhaps just be an array of strings.
Also, it seems that three of the four calendar methods are nearly identical other than the class of calendar object created: generateGoogleCalendar, generateOutlookCalendar and generateYahooCalendar. A generic method could be used to replace those three methods (and accept the calendar type as a parameter to be used to dynamically invoke a constructor) or create them dynamically in a loop.